From 43f1f145912af57388eafafdac4a3f0330e5a2e4 Mon Sep 17 00:00:00 2001 From: Rishabh Goyal Date: Sun, 13 Apr 2025 02:35:23 +0530 Subject: [PATCH 1/3] Few optimizations in performance --- pom.xml | 57 +++---------------- .../engine/DataSetAccessor.java | 13 +++-- .../engine/ExecutionGraphGenerator.java | 4 +- .../engine/SimpleDataFlowExecutor.java | 18 +++--- .../databuilderframework/engine/Utils.java | 4 +- .../model/DataBuilderMeta.java | 47 ++++++++++----- .../controller/HomePageControllerTest.java | 3 +- 7 files changed, 63 insertions(+), 83 deletions(-) diff --git a/pom.xml b/pom.xml index 95cd532..0100dbe 100644 --- a/pom.xml +++ b/pom.xml @@ -62,8 +62,7 @@ UTF-8 2.10.3 - 1.16.6 - 1.16.6.1 + 1.18.30 5.2.5.Final 4.13.1 @@ -93,6 +92,10 @@ ${hibernate.validator.version} provided + + org.apache.commons + commons-collections4 + 4.4 org.slf4j log4j-over-slf4j @@ -117,60 +120,14 @@ - - org.projectlombok - lombok-maven-plugin - - - generate-sources - - delombok - - - - ${lombok.maven.version} - org.apache.maven.plugins maven-compiler-plugin - 1.8 - 1.8 + 17 + 17 - - org.codehaus.mojo - cobertura-maven-plugin - 2.6 - - - 85 - 85 - true - 85 - 85 - 85 - 85 - - - com.flipkart.databuilderframework.* - 100 - 100 - - - - - true - - - - - - clean - - - - org.apache.maven.plugins maven-jar-plugin diff --git a/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java b/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java index 200d150..2ada206 100644 --- a/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java +++ b/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java @@ -35,8 +35,8 @@ public T get(Class tClass) { */ public T get(String key, Class tClass) { Map availableData = dataSet.getAvailableData(); - if (availableData.containsKey(key)) { - Data data = availableData.get(key); + Data data = availableData.get(key); + if (data != null) { return tClass.cast(data); } return null; @@ -76,7 +76,7 @@ public DataSet getAccesibleDataSetFor(DataBuilder builder) { * @param data {@link com.flipkart.databuilderframework.model.Data} to be merged. */ public void merge(Data data) { - dataSet.getAvailableData().put(data.getData(), data); + dataSet.add(data.getData(), data); } /** @@ -99,7 +99,12 @@ public void merge(DataDelta dataDelta) { */ public boolean checkForData(Set dataList) { Map availableData = dataSet.getAvailableData(); - return availableData.keySet().containsAll(dataList); + for (String key : dataList) { + if (!availableData.containsKey(key)) { + return false; + } + } + return true; } /** diff --git a/src/main/java/com/flipkart/databuilderframework/engine/ExecutionGraphGenerator.java b/src/main/java/com/flipkart/databuilderframework/engine/ExecutionGraphGenerator.java index 348fd02..c68e25c 100644 --- a/src/main/java/com/flipkart/databuilderframework/engine/ExecutionGraphGenerator.java +++ b/src/main/java/com/flipkart/databuilderframework/engine/ExecutionGraphGenerator.java @@ -100,10 +100,8 @@ private List> buildHierarchy( //Data is user-input data continue; } - DataBuilderMeta dataBuilderMeta = tmpDataBuilderMeta.deepCopy(); int rank = dependencyInfo.getValue().getRank(); - dataBuilderMeta.setRank(rank); - + DataBuilderMeta dataBuilderMeta = tmpDataBuilderMeta.deepCopy(rank); //Set builder in the appropriate rank slots if(null == dependencyHierarchy.get(rank)) { dependencyHierarchy.set(rank, Lists.newArrayList()); diff --git a/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java b/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java index 83b5212..ff1ee2d 100644 --- a/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java +++ b/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java @@ -4,6 +4,8 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import lombok.val; +import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,13 +41,13 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext, DataSet dataSet = dataFlowInstance.getDataSet().accessor().copy(); //Create own copy to work with DataSetAccessor dataSetAccessor = DataSet.accessor(dataSet); dataSetAccessor.merge(dataDelta); - Map responseData = Maps.newTreeMap(); + Map responseData = Maps.newHashMap(); Set activeDataSet = Sets.newHashSet(); - activeDataSet.addAll(dataDelta.getDelta() - .stream() - .map(Data::getData) - .collect(Collectors.toList())); + for (Data deltaDeltaElement : dataDelta.getDelta()) { + activeDataSet.add(deltaDeltaElement.getData()); + } + List> dependencyHierarchy = executionGraph.getDependencyHierarchy(); Set newlyGeneratedData = Sets.newHashSet(); Set processedBuilders = Sets.newHashSet(); @@ -55,10 +57,12 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext, if (processedBuilders.contains(builderMeta)) { continue; } - //If there is an intersection, means some of it's inputs have changed. Reevaluate - if (Sets.intersection(builderMeta.getEffectiveConsumes(), activeDataSet).isEmpty()) { + + val effectiveConsumes = builderMeta.getEffectiveConsumes(); + if (!CollectionUtils.containsAny(effectiveConsumes, activeDataSet)) { continue; } + DataBuilder builder = builderFactory.create(builderMeta); if (!dataSetAccessor.checkForData(builder.getDataBuilderMeta().getConsumes())) { continue; diff --git a/src/main/java/com/flipkart/databuilderframework/engine/Utils.java b/src/main/java/com/flipkart/databuilderframework/engine/Utils.java index 569a02a..2d0ba98 100644 --- a/src/main/java/com/flipkart/databuilderframework/engine/Utils.java +++ b/src/main/java/com/flipkart/databuilderframework/engine/Utils.java @@ -30,8 +30,8 @@ public static String name(Object object) { } public static String name(Class clazz) { - return CaseFormat.UPPER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, - CLASS_TO_NAME_MAPPING.computeIfAbsent(clazz, aClass -> clazz.getSimpleName())); + return CLASS_TO_NAME_MAPPING.computeIfAbsent(clazz, aClass -> + CaseFormat.UPPER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, clazz.getSimpleName())); } public static boolean isEmpty(Collection collection) { diff --git a/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java b/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java index 80fab68..4ea8f13 100644 --- a/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java +++ b/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import lombok.Builder; +import lombok.EqualsAndHashCode; import org.hibernate.validator.constraints.NotEmpty; import javax.validation.constraints.NotNull; @@ -18,6 +19,7 @@ * execution. */ @lombok.Data +@EqualsAndHashCode(cacheStrategy = EqualsAndHashCode.CacheStrategy.LAZY) public class DataBuilderMeta implements Comparable, Serializable { /** * List of {@link com.flipkart.databuilderframework.model.Data} this {@link com.flipkart.databuilderframework.engine.DataBuilder} @@ -26,7 +28,7 @@ public class DataBuilderMeta implements Comparable, Serializabl @NotNull @NotEmpty @JsonProperty - private Set consumes; + private final Set consumes; /** * {@link com.flipkart.databuilderframework.model.Data} this {@link com.flipkart.databuilderframework.engine.DataBuilder} generates. @@ -34,7 +36,7 @@ public class DataBuilderMeta implements Comparable, Serializabl @NotNull @NotEmpty @JsonProperty - private String produces; + private final String produces; /** * Name for this builder @@ -42,9 +44,9 @@ public class DataBuilderMeta implements Comparable, Serializabl @NotNull @NotEmpty @JsonProperty - private String name; + private final String name; - private int rank; + private final int rank; /** @@ -54,7 +56,7 @@ public class DataBuilderMeta implements Comparable, Serializabl * all consumes {@link com.flipkart.databuilderframework.model.Data} are present but its optional and not mandatory for {@link com.flipkart.databuilderframework.engine.DataBuilder} to run. */ @JsonProperty - private Set optionals; + private final Set optionals; /** * Set of {@link com.flipkart.databuilderframework.model.Data} this {@link com.flipkart.databuilderframework.engine.DataBuilder} @@ -63,28 +65,37 @@ public class DataBuilderMeta implements Comparable, Serializabl @NotNull @NotEmpty @JsonProperty - private Set access; + private final Set access; + + private final Set accessibleDataSet; + + private final Set effectiveConsumes; public DataBuilderMeta(Set consumes, String produces, String name) { - this(consumes, produces, name, Collections.emptySet(), Collections.emptySet()); + this(consumes, produces, name, 0, Collections.emptySet(), Collections.emptySet()); } @Builder - public DataBuilderMeta(Set consumes, String produces, String name, + public DataBuilderMeta(Set consumes, String produces, String name, + Set optionals, Set access) { + this(consumes, produces, name, 0, optionals, access); + } + + public DataBuilderMeta(Set consumes, String produces, String name, + int rank, Set optionals, Set access) { this.consumes = consumes; this.produces = produces; this.name = name; + this.rank = rank; this.optionals = optionals; this.access = access; - } - - - public DataBuilderMeta() { + this.accessibleDataSet = getAllAccessibleDataSet(); + this.effectiveConsumes = getAllEffectiveConsumes(); } @JsonIgnore - public Set getEffectiveConsumes(){ + private Set getAllEffectiveConsumes(){ if(optionals != null && !optionals.isEmpty()){ return Sets.union(optionals, consumes); }else{ @@ -93,9 +104,9 @@ public Set getEffectiveConsumes(){ } @JsonIgnore - public Set getAccessibleDataSet(){ + private Set getAllAccessibleDataSet(){ Set output = consumes; - if(optionals != null && !optionals.isEmpty()){ + if(optionals != null && !optionals.isEmpty()) { output = Sets.union(optionals, output); } if(access != null && !access.isEmpty()){ @@ -113,4 +124,10 @@ public DataBuilderMeta deepCopy() { Set accessCopy = (access != null) ? ImmutableSet.copyOf(access) : null; return new DataBuilderMeta(ImmutableSet.copyOf(consumes), produces, name, optionalCopy, accessCopy); } + + public DataBuilderMeta deepCopy(int rank) { + Set optionalCopy = (optionals != null) ? ImmutableSet.copyOf(optionals) : null; + Set accessCopy = (access != null) ? ImmutableSet.copyOf(access) : null; + return new DataBuilderMeta(ImmutableSet.copyOf(consumes), produces, name, rank, optionalCopy, accessCopy); + } } diff --git a/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java b/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java index 3eef0b6..0a3a3f6 100644 --- a/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java +++ b/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java @@ -46,10 +46,9 @@ public void testHomePageMT() throws Exception { private void runHomePageTest(DataFlowExecutor executor) throws Exception { final HomePageRequest request = new HomePageRequest("2321312312", "2323454", "Blah".getBytes()); Stopwatch stopwatch = Stopwatch.createStarted(); - for(long i = 0; i < 100000; i++) { + for(long i = 0; i < 10000000; i++) { HomePageResponse response = executor.run(homePageDataFlow, request).get(HomePageResponse.class); Assert.assertNotNull(response); - //System.out.println(new ObjectMapper().writerWithDefaultPrettyPrinter().writeValueAsString(response)); } log.info("Time taken: {}", stopwatch.elapsed(TimeUnit.MILLISECONDS)); } From 6d35539468a8a2934cf39648012856bd8bb1e8dd Mon Sep 17 00:00:00 2001 From: Rishabh Goyal Date: Mon, 14 Apr 2025 16:13:58 +0530 Subject: [PATCH 2/3] Updated compiler version --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index ddcfd4a..59ef6bf 100644 --- a/pom.xml +++ b/pom.xml @@ -143,8 +143,8 @@ org.apache.maven.plugins maven-compiler-plugin - 8 - 8 + 1.8 + 1.8 From 6ea121e9f5045972cacc00aab89c60228d14c841 Mon Sep 17 00:00:00 2001 From: Rishabh Goyal Date: Mon, 14 Apr 2025 17:24:20 +0530 Subject: [PATCH 3/3] Reverted few changes --- pom.xml | 4 ---- .../databuilderframework/engine/DataSetAccessor.java | 3 ++- .../engine/SimpleDataFlowExecutor.java | 10 ++-------- .../databuilderframework/model/DataBuilderMeta.java | 12 +++++------- .../controller/HomePageControllerTest.java | 1 + 5 files changed, 10 insertions(+), 20 deletions(-) diff --git a/pom.xml b/pom.xml index 59ef6bf..a6b6413 100644 --- a/pom.xml +++ b/pom.xml @@ -92,10 +92,6 @@ ${hibernate.validator.version} provided - - org.apache.commons - commons-collections4 - 4.4 org.slf4j log4j-over-slf4j diff --git a/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java b/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java index 9bf00d5..50ce7c6 100644 --- a/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java +++ b/src/main/java/com/flipkart/databuilderframework/engine/DataSetAccessor.java @@ -73,10 +73,11 @@ public DataSet getAccesibleDataSetFor(DataBuilder builder) { * Merge data with current {@link com.flipkart.databuilderframework.model.DataSet}. * Data will be added to the current data-set and override data with same type as identified by * {@link com.flipkart.databuilderframework.model.Data#getData()} + * * @param data {@link com.flipkart.databuilderframework.model.Data} to be merged. */ public void merge(Data data) { - dataSet.add(data.getData(), data); + dataSet.getAvailableData().put(data.getData(), data); } /** diff --git a/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java b/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java index 3a61099..78a718d 100644 --- a/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java +++ b/src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java @@ -4,8 +4,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import lombok.val; -import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -13,7 +11,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; /** * The executor for a {@link com.flipkart.databuilderframework.model.DataFlow}. @@ -47,7 +44,6 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext, for (Data deltaDeltaElement : dataDelta.getDelta()) { activeDataSet.add(deltaDeltaElement.getData()); } - List> dependencyHierarchy = executionGraph.getDependencyHierarchy(); Set newlyGeneratedData = Sets.newHashSet(); Set processedBuilders = Sets.newHashSet(); @@ -57,12 +53,10 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext, if (processedBuilders.contains(builderMeta)) { continue; } - - val effectiveConsumes = builderMeta.getEffectiveConsumes(); - if (!CollectionUtils.containsAny(effectiveConsumes, activeDataSet)) { + //If there is an intersection, means some of it's inputs have changed. Reevaluate + if (Sets.intersection(builderMeta.getEffectiveConsumes(), activeDataSet).isEmpty()) { continue; } - DataBuilder builder = builderFactory.create(builderMeta); if (!dataSetAccessor.checkForData(builder.getDataBuilderMeta().getConsumes())) { continue; diff --git a/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java b/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java index 4ea8f13..5ab7aa5 100644 --- a/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java +++ b/src/main/java/com/flipkart/databuilderframework/model/DataBuilderMeta.java @@ -90,12 +90,12 @@ public DataBuilderMeta(Set consumes, String produces, String name, this.rank = rank; this.optionals = optionals; this.access = access; - this.accessibleDataSet = getAllAccessibleDataSet(); - this.effectiveConsumes = getAllEffectiveConsumes(); + this.accessibleDataSet = constructAccessibleDataSet(); + this.effectiveConsumes = constructEffectiveConsumes(); } @JsonIgnore - private Set getAllEffectiveConsumes(){ + private Set constructEffectiveConsumes(){ if(optionals != null && !optionals.isEmpty()){ return Sets.union(optionals, consumes); }else{ @@ -104,7 +104,7 @@ private Set getAllEffectiveConsumes(){ } @JsonIgnore - private Set getAllAccessibleDataSet(){ + private Set constructAccessibleDataSet(){ Set output = consumes; if(optionals != null && !optionals.isEmpty()) { output = Sets.union(optionals, output); @@ -120,9 +120,7 @@ public int compareTo(DataBuilderMeta rhs) { } public DataBuilderMeta deepCopy() { - Set optionalCopy = (optionals != null) ? ImmutableSet.copyOf(optionals) : null; - Set accessCopy = (access != null) ? ImmutableSet.copyOf(access) : null; - return new DataBuilderMeta(ImmutableSet.copyOf(consumes), produces, name, optionalCopy, accessCopy); + return deepCopy(0); } public DataBuilderMeta deepCopy(int rank) { diff --git a/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java b/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java index 5046742..2e7f0e5 100644 --- a/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java +++ b/src/test/java/examples/bloghomepagebuilder/controller/HomePageControllerTest.java @@ -49,6 +49,7 @@ private void runHomePageTest(DataFlowExecutor executor) throws Exception { for(long i = 0; i < 1000000; i++) { HomePageResponse response = executor.run(homePageDataFlow, request).get(HomePageResponse.class); Assert.assertNotNull(response); + //System.out.println(new ObjectMapper().writerWithDefaultPrettyPrinter().writeValueAsString(response)); } log.info("Time taken: {}", stopwatch.elapsed(TimeUnit.MILLISECONDS)); }