Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Reduce RTS calls in the updateLayout flow #37127

Merged
merged 47 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
0c4b37b
Revert "chore: removed unwanted metrics (#37052)"
rishabhrathod01 Oct 29, 2024
a150314
chore: Add metrics to analyse RTS logs
rishabhrathod01 Oct 29, 2024
448f663
Send all bindings together to avoid multiple RTS calls
rishabhrathod01 Oct 29, 2024
e3ec3c0
remove commented code
rishabhrathod01 Oct 31, 2024
65b5492
undo change to verify ci failure cause
rishabhrathod01 Oct 31, 2024
87ffad7
undo change to verify ci failure cause
rishabhrathod01 Nov 1, 2024
28a92e1
fix cypress test
rishabhrathod01 Nov 1, 2024
42a274d
use constants for span
rishabhrathod01 Nov 1, 2024
cfa0d94
undo changes in method to be skipped
rishabhrathod01 Nov 8, 2024
41209f5
undo addDirectly.. method change
rishabhrathod01 Nov 11, 2024
9b3b5f4
send allBindings to getPossibleEntityReferences
rishabhrathod01 Nov 12, 2024
fa2ed2c
remove unused code
rishabhrathod01 Nov 12, 2024
e9fe711
Merge branch 'release' into chore/rts-metrics
Nov 12, 2024
e108c96
trigger CI
rishabhrathod01 Nov 12, 2024
2b0981b
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 12, 2024
2a8d811
avoid similar binding leading to issues
rishabhrathod01 Nov 18, 2024
55120d3
add unit test
rishabhrathod01 Nov 18, 2024
bae1fec
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 19, 2024
8e69ab5
fix unit test
rishabhrathod01 Nov 19, 2024
b55d0a8
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 19, 2024
b732496
add log to debug on ci
rishabhrathod01 Nov 19, 2024
6a42adc
print log to debug unit test
rishabhrathod01 Nov 19, 2024
edb933f
print log to debug unit test
rishabhrathod01 Nov 20, 2024
45000b1
fix unit test
rishabhrathod01 Nov 21, 2024
55c9586
disable migrateDSL when fetching dsl in unit test
rishabhrathod01 Nov 21, 2024
790caa6
resolve comments
rishabhrathod01 Nov 21, 2024
afc92f3
extract common code to reduce code duplicacy
rishabhrathod01 Nov 21, 2024
72a4022
refactor and add comment
rishabhrathod01 Nov 21, 2024
b532bc6
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 21, 2024
4da0054
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 21, 2024
4206960
remove unrequired constructor
rishabhrathod01 Nov 22, 2024
29eb2d0
undo common code separation
rishabhrathod01 Nov 24, 2024
e0a2bda
undo changes
rishabhrathod01 Nov 25, 2024
22c00cb
spotless apply
rishabhrathod01 Nov 25, 2024
ae70aa5
fix method type
rishabhrathod01 Nov 25, 2024
fc5ef6a
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 25, 2024
bdd2e7f
simplify logic to be more readable
rishabhrathod01 Nov 26, 2024
98b7eb5
mock unit test for allBindings
rishabhrathod01 Nov 26, 2024
5e4ffe1
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 26, 2024
191e96f
return edgesRef like before
rishabhrathod01 Nov 27, 2024
a6d2b46
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 27, 2024
d1b3791
Revert "simplify logic to be more readable"
rishabhrathod01 Nov 27, 2024
75210b1
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 27, 2024
01e13b7
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 28, 2024
da8b8d6
remove unused span
rishabhrathod01 Nov 28, 2024
f58d22b
retain comment
rishabhrathod01 Nov 28, 2024
07b68d4
Merge branch 'release' into chore/rts-metrics
rishabhrathod01 Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ public class OnLoadSpanCE {
APPSMITH_SPAN_PREFIX + "getAllExecutablesByCreatorIdFlux";
public static final String EXECUTABLE_NAME_TO_EXECUTABLE_MAP =
APPSMITH_SPAN_PREFIX + "executableNameToExecutableMap";
public static final String EXECUTABLE_IN_CREATOR_CONTEXT = APPSMITH_SPAN_PREFIX + "executablesInCreatorContext";
public static final String ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH =
APPSMITH_SPAN_PREFIX + "addDirectlyReferencedExecutablesToGraph";
public static final String UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,9 +524,8 @@ private Mono<Set<EntityDependencyNode>> getPossibleEntityReferences(
Set<EntityDependencyNode> bindingsInDsl) {
// We want to be finding both type of references
final int entityTypes = EXECUTABLE_ENTITY_REFERENCES | WIDGET_ENTITY_REFERENCES;

return executableNameToExecutableMono
.zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion))
.zipWith(getPossibleEntityParentsMap(new ArrayList<>(bindings), entityTypes, evalVersion))
.map(tuple -> {
Map<String, Executable> executableMap = tuple.getT1();
// For each binding, here we receive a set of possible references to global entities
Expand Down Expand Up @@ -584,6 +583,79 @@ private Mono<Set<EntityDependencyNode>> getPossibleEntityReferences(
});
}

private Mono<Map<String, Set<EntityDependencyNode>>> getPossibleEntityReferencesMap(
Mono<Map<String, Executable>> executableNameToExecutableMono,
List<String> bindings,
int evalVersion,
Set<EntityDependencyNode> bindingsInDsl) {
// We want to be finding both type of references
final int entityTypes = EXECUTABLE_ENTITY_REFERENCES | WIDGET_ENTITY_REFERENCES;

return executableNameToExecutableMono
.zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion))
.map(tuple -> {
Map<String, Executable> executableMap = tuple.getT1();
// For each binding, here we receive a set of possible references to global entities
// At this point we're guaranteed that these references are made to possible variables,
// but we do not know if those entities exist in the global namespace yet
Map<String, Set<EntityDependencyNode>> bindingToPossibleParentMap = tuple.getT2();

Map<String, Set<EntityDependencyNode>> possibleEntitiesReferencesToBindingMap = new HashMap<>();

// From these references, we will try to validate executable references at this point
// Each identified node is already annotated with the expected type of entity we need to search for
bindingToPossibleParentMap.entrySet().stream().forEach(entry -> {
Set<EntityDependencyNode> bindingsWithExecutableReference = new HashSet<>();
String binding = entry.getKey();
Set<EntityDependencyNode> possibleEntitiesReferences = new HashSet<>();
entry.getValue().stream().forEach(possibleParent -> {
// For each possible reference node, check if the reference was to an executable
Executable executable = executableMap.get(possibleParent.getValidEntityName());

if (executable != null) {
// If it was, and had been identified as the same type of executable as what exists in
// this app,
if (possibleParent
.getEntityReferenceType()
.equals(executable.getEntityReferenceType())) {
// Copy over some data from the identified executable, this ensures that we do not
// have
// to query the DB again later
possibleParent.setExecutable(executable);
bindingsWithExecutableReference.add(possibleParent);
// Only if this is not a direct JS function call,
// add it to a possible on page load executable call.
// This discards the following type:
// {{ JSObject1.func() }}
if (!TRUE.equals(possibleParent.getIsFunctionCall())) {
possibleEntitiesReferences.add(possibleParent);
}
// We're ignoring any reference that was identified as a widget but actually matched
// an executable
// We wouldn't have discarded JS collection names here, but this is just an
// optimization, so it's fine
}
} else {
// If the reference node was identified as a widget, directly add it as a possible
// reference
// Because we are not doing any validations for widget references at this point
if (EntityReferenceType.WIDGET.equals(possibleParent.getEntityReferenceType())) {
possibleEntitiesReferences.add(possibleParent);
}
}

possibleEntitiesReferencesToBindingMap.put(binding, possibleEntitiesReferences);
});

if (!bindingsWithExecutableReference.isEmpty() && bindingsInDsl != null) {
bindingsInDsl.addAll(bindingsWithExecutableReference);
}
});

return possibleEntitiesReferencesToBindingMap;
});
}

/**
* This method is an abstraction that queries the ast service for possible global references as string values,
* and then uses the mustache helper utility to classify these global references into possible types of EntityDependencyNodes
Expand All @@ -594,9 +666,9 @@ private Mono<Set<EntityDependencyNode>> getPossibleEntityReferences(
* @return A mono of a map of each of the provided binding values to the possible set of EntityDependencyNodes found in the binding
*/
private Mono<Map<String, Set<EntityDependencyNode>>> getPossibleEntityParentsMap(
Set<String> bindings, int types, int evalVersion) {
List<String> bindings, int types, int evalVersion) {
Flux<Tuple2<String, Set<String>>> findingToReferencesFlux =
astService.getPossibleReferencesFromDynamicBinding(new ArrayList<>(bindings), evalVersion);
astService.getPossibleReferencesFromDynamicBinding(bindings, evalVersion);
return MustacheHelper.getPossibleEntityParentsMap(findingToReferencesFlux, types);
}

Expand Down Expand Up @@ -627,31 +699,43 @@ private Mono<Set<ExecutableDependencyEdge>> addDirectlyReferencedExecutablesToGr
Mono<Map<String, Executable>> executableNameToExecutableMapMono,
Set<EntityDependencyNode> executableBindingsInDslRef,
int evalVersion) {
return Flux.fromIterable(widgetDynamicBindingsMap.entrySet())
.flatMap(entry -> {
String widgetName = entry.getKey();
// For each widget in the DSL that has a dynamic binding,
// we define an entity dependency node beforehand
// This will be a leaf node in the DAG that is constructed for on page load dependencies
EntityDependencyNode widgetDependencyNode =
new EntityDependencyNode(EntityReferenceType.WIDGET, widgetName, widgetName, null, null);
Set<String> bindingsInWidget = entry.getValue();
return getPossibleEntityReferences(
executableNameToExecutableMapMono,
bindingsInWidget,
evalVersion,
executableBindingsInDslRef)
.flatMapMany(Flux::fromIterable)
// Add dependencies of the executables found in the DSL in the graph
// We are ignoring the widget references at this point
// TODO: Possible optimization in the future
.flatMap(possibleEntity -> {

Map<String, Set<EntityDependencyNode>> bindingToWidgetNodesMap = new HashMap<>();
List<String> allBindings = new ArrayList<>();

widgetDynamicBindingsMap.forEach((widgetName, bindingsInWidget) -> {
EntityDependencyNode widgetDependencyNode =
new EntityDependencyNode(EntityReferenceType.WIDGET, widgetName, widgetName, null, null);

bindingsInWidget.forEach(binding -> {
bindingToWidgetNodesMap
.computeIfAbsent(binding, bindingKey -> new HashSet<>())
.add(widgetDependencyNode);
allBindings.add(binding);
});
});

Mono<Map<String, Set<EntityDependencyNode>>> bindingToPossibleEntityMapMono = getPossibleEntityReferencesMap(
executableNameToExecutableMapMono, allBindings, evalVersion, executableBindingsInDslRef);

return bindingToPossibleEntityMapMono
.flatMapMany(bindingToPossibleEntityMap -> Flux.fromIterable(bindingToPossibleEntityMap.entrySet()))
.flatMap(bindingEntry -> {
String binding = bindingEntry.getKey();
Set<EntityDependencyNode> possibleEntities = bindingEntry.getValue();

// Get all widget nodes associated with the binding
Set<EntityDependencyNode> widgetDependencyNodes =
bindingToWidgetNodesMap.getOrDefault(binding, Set.of());

// Process each possibleEntity for the current binding
return Flux.fromIterable(possibleEntities).flatMap(possibleEntity -> Flux.fromIterable(
widgetDependencyNodes) // Iterate all associated widgets
.flatMap(widgetDependencyNode -> {
if (getExecutableTypes().contains(possibleEntity.getEntityReferenceType())) {
edgesRef.add(new ExecutableDependencyEdge(possibleEntity, widgetDependencyNode));
// This executable is directly referenced in the DSL. This executable is an ideal
// candidate
// for on page load
executablesUsedInDSLRef.add(possibleEntity.getValidEntityName());

return updateExecutableSelfReferencingPaths(possibleEntity)
.name(UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS)
.tap(Micrometer.observation(observationRegistry))
Expand All @@ -668,7 +752,7 @@ private Mono<Set<ExecutableDependencyEdge>> addDirectlyReferencedExecutablesToGr
.thenReturn(possibleEntity);
}
return Mono.just(possibleEntity);
});
}));
})
.collectList()
.thenReturn(edgesRef);
Expand Down Expand Up @@ -1134,7 +1218,7 @@ private Mono<Set<ExecutableDependencyEdge>> addWidgetRelationshipToGraph(
// This part will ensure that we are discovering widget to widget relationships.
return Flux.fromIterable(widgetBindingMap.entrySet())
.flatMap(widgetBindingEntries -> getPossibleEntityParentsMap(
widgetBindingEntries.getValue(), entityTypes, evalVersion)
new ArrayList<>(widgetBindingEntries.getValue()), entityTypes, evalVersion)
.map(possibleParentsMap -> {
possibleParentsMap.entrySet().stream().forEach(entry -> {
if (entry.getValue() == null || entry.getValue().isEmpty()) {
Expand Down
Loading
Loading