Merge pull request #364 from commercetools/246-improve-b

246 improve benchmarks

Author: heshamMassoud

.travis.yml

@@ -13,20 +13,8 @@ before_install:
 install: true # skips travis' default installation step which executes gradle assemble.
 jobs:
   include:
-    - stage: benchmark#1
-      if: tag IS present
-      script: ./gradlew clean setLibraryVersion benchmark benchmarkCommit
-    - stage: benchmark#2
-      if: tag IS present
-      script: ./gradlew clean setLibraryVersion benchmark benchmarkCommit
-    - stage: benchmark#3
-      if: tag IS present
-      script: ./gradlew clean setLibraryVersion benchmark benchmarkCommit
-    - stage: benchmark#4
-      if: tag IS present
-      script: ./gradlew clean setLibraryVersion benchmark benchmarkCommit
-    - stage: benchmark#5
-      if: tag IS present
+    - stage: benchmark
+      if: branch = master AND commit_message =~ /^(Merge pull request .*)/
       script: ./gradlew clean setLibraryVersion benchmark benchmarkCommit
     - stage: full build
       script: ./gradlew clean dependencyUpdates setLibraryVersion build

docs/RELEASE_NOTES.md

@@ -47,6 +47,9 @@
     now treat the values of the optional fields `isSearchable`, `inputHint` and `attributeConstraint` 
     as (`true`, `SingleLine` and `None` respectivley) if they are `null` or not passed. [#354](https://github.com/commercetools/commercetools-sync-java/issues/354)
 
+- 🛠️ **Enhancements** (1)
+    - **Commons** - Benchmarks are now run once on every merge to `master` with a lower number of resources for faster benchmarking. [#246](https://github.com/commercetools/commercetools-sync-java/issues/246)
+
 - 📋 **Documentation** (2)
     - **Commons** - Added link to [documentation pages](https://commercetools.github.io/commercetools-sync-java) in README of the github repo.
     - **Commons** - Fixed link of [`beforeUpdateCallback` for keeping other variants](https://github.com/commercetools/commercetools-sync-java/tree/master/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/products/templates/beforeupdatecallback/KeepOtherVariantsSyncIT.java) example in the [Sync Options](/docs/usage/SYNC_OPTIONS.md) doc page. [#360](https://github.com/commercetools/commercetools-sync-java/issues/360)

src/benchmark/java/com/commercetools/sync/benchmark/BenchmarkUtils.java

@@ -11,26 +11,18 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Optional;
-import java.util.Spliterator;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import static java.util.Optional.ofNullable;
-import static java.util.Spliterators.spliteratorUnknownSize;
-import static java.util.stream.StreamSupport.stream;
 
-public class BenchmarkUtils {
+final class BenchmarkUtils {
     private static final String BENCHMARK_RESULTS_FILE_NAME = "benchmarks.json";
     private static final String BENCHMARK_RESULTS_FILE_DIR = ofNullable(System.getenv("CI_BUILD_DIR"))
         .map(path -> path + "/tmp_git_dir/benchmarks/").orElse("");
     private static final String BENCHMARK_RESULTS_FILE_PATH = BENCHMARK_RESULTS_FILE_DIR + BENCHMARK_RESULTS_FILE_NAME;
     private static final Charset UTF8_CHARSET = StandardCharsets.UTF_8;
-    private static final String EXECUTION_TIMES = "executionTimes";
-    private static final String AVERAGE = "average";
-    private static final String DIFF = "diff";
+    private static final String EXECUTION_TIME = "executionTime";
+    private static final String BRANCH_NAME = ofNullable(System.getenv("TRAVIS_COMMIT"))
+        .orElse("dev-local");
 
     static final String PRODUCT_SYNC = "productSync";
     static final String INVENTORY_SYNC = "inventorySync";
@@ -40,137 +32,86 @@ public class BenchmarkUtils {
     static final String CREATES_ONLY = "createsOnly";
     static final String UPDATES_ONLY = "updatesOnly";
     static final String CREATES_AND_UPDATES = "mix";
-    static final int THRESHOLD = 120000; //120 seconds in milliseconds
-    static final int NUMBER_OF_RESOURCE_UNDER_TEST = 10000;
+    static final int NUMBER_OF_RESOURCE_UNDER_TEST = 1000;
+    static final String THRESHOLD_EXCEEDED_ERROR = "Total execution time of benchmark '%d' took longer than allowed"
+        + " threshold of '%d'.";
 
 
-    static void saveNewResult(@Nonnull final String version,
-                              @Nonnull final String sync,
-                              @Nonnull final String benchmark,
-                              final double newResult) throws IOException {
+    static void saveNewResult(@Nonnull final String sync, @Nonnull final String benchmark, final double newResult)
+        throws IOException {
 
-        final JsonNode rootNode = new ObjectMapper().readTree(getFileContent(BENCHMARK_RESULTS_FILE_PATH));
-        final JsonNode withNewResult = addNewResult(rootNode, version, sync, benchmark, newResult);
-        writeToFile(withNewResult.toString(), BENCHMARK_RESULTS_FILE_PATH);
+        final JsonNode rootNode = new ObjectMapper().readTree(getFileContent());
+        final JsonNode withNewResult = addNewResult(rootNode, sync, benchmark, newResult);
+        writeToFile(withNewResult.toString());
     }
 
+    @Nonnull
+    private static String getFileContent() throws IOException {
+
+        final byte[] fileBytes = Files.readAllBytes(Paths.get(BENCHMARK_RESULTS_FILE_PATH));
+        return new String(fileBytes, UTF8_CHARSET);
+    }
+
+    @Nonnull
     private static JsonNode addNewResult(@Nonnull final JsonNode originalRoot,
-                                         @Nonnull final String version,
                                          @Nonnull final String sync,
                                          @Nonnull final String benchmark,
                                          final double newResult) {
+
         ObjectNode rootNode = (ObjectNode) originalRoot;
-        ObjectNode versionNode = (ObjectNode) rootNode.get(version);
+        ObjectNode branchNode = (ObjectNode) rootNode.get(BRANCH_NAME);
 
         // If version doesn't exist yet, create a new JSON object for the new version.
-        if (versionNode == null) {
-            rootNode = createVersionNode(rootNode, version);
-            versionNode = (ObjectNode) rootNode.get(version);
+        if (branchNode == null) {
+            branchNode = createVersionNode();
+            rootNode.set(BRANCH_NAME, branchNode);
         }
 
-        final ObjectNode syncNode = (ObjectNode) versionNode.get(sync);
+        final ObjectNode syncNode = (ObjectNode) branchNode.get(sync);
         final ObjectNode benchmarkNode = (ObjectNode) syncNode.get(benchmark);
 
-        // Get current list of execution times for the specified benchmark of the specified sync module
-        // of the specified version.
-        final List<JsonNode> results = toList(benchmarkNode.get(EXECUTION_TIMES).elements());
-
         // Add new result.
-        results.add(JsonNodeFactory.instance.numberNode(newResult));
-        benchmarkNode.set(EXECUTION_TIMES, JsonNodeFactory.instance.arrayNode().addAll(results));
-
-        // Compute new average and add to JSON Object
-        final double averageResult = calculateAvg(results);
-        benchmarkNode.set(AVERAGE, JsonNodeFactory.instance.numberNode(averageResult));
-
-        // Compute new diff from the last version.
-        final double diff = calculateDiff(rootNode, version, sync, benchmark, averageResult);
-        benchmarkNode.set(DIFF, JsonNodeFactory.instance.numberNode(diff));
-
-        final JsonNode newSyncNode = syncNode.set(benchmark, benchmarkNode);
-        final JsonNode newVersionNode = versionNode.set(sync, newSyncNode);
-        final JsonNode newRoot = rootNode.set(version, newVersionNode);
-        return newRoot;
+        benchmarkNode.set(EXECUTION_TIME, JsonNodeFactory.instance.numberNode(newResult));
+        return rootNode;
     }
 
     @Nonnull
-    private static <T> List<T> toList(@Nonnull final Iterator<T> iterator) {
-        return toStream(iterator).collect(Collectors.toList());
-    }
+    private static ObjectNode createVersionNode() {
 
-    @Nonnull
-    private static <T> Stream<T> toStream(@Nonnull final Iterator<T> iterator) {
-        return stream(spliteratorUnknownSize(iterator, Spliterator.ORDERED), false);
-    }
-
-    private static double calculateAvg(@Nonnull final List<JsonNode> results) {
-        return results.stream().mapToDouble(JsonNode::asLong).average().orElse(0);
-    }
+        final ObjectNode newVersionNode = JsonNodeFactory.instance.objectNode();
+        newVersionNode.set(PRODUCT_SYNC, createSyncNode());
+        newVersionNode.set(INVENTORY_SYNC, createSyncNode());
+        newVersionNode.set(CATEGORY_SYNC, createSyncNode());
+        newVersionNode.set(PRODUCT_TYPE_SYNC, createSyncNode());
+        newVersionNode.set(TYPE_SYNC, createSyncNode());
 
-    private static ObjectNode createVersionNode(@Nonnull final ObjectNode rootNode, @Nonnull final String version) {
-        final ObjectNode newVersionNode = createSyncNode(createSyncNode(createSyncNode(
-            createSyncNode(createSyncNode(JsonNodeFactory.instance.objectNode(),
-                PRODUCT_SYNC), INVENTORY_SYNC), CATEGORY_SYNC), PRODUCT_TYPE_SYNC), TYPE_SYNC);
-        return (ObjectNode) rootNode.set(version, newVersionNode);
+        return newVersionNode;
     }
 
-    private static ObjectNode createSyncNode(@Nonnull final ObjectNode versionNode,
-                                             @Nonnull final String sync) {
-        final ObjectNode newSyncNode = createBenchmarkNode(
-            createBenchmarkNode(
-                createBenchmarkNode(JsonNodeFactory.instance.objectNode(), CREATES_ONLY), UPDATES_ONLY),
-            CREATES_AND_UPDATES);
-        return (ObjectNode) versionNode.set(sync, newSyncNode);
-    }
+    @Nonnull
+    private static ObjectNode createSyncNode() {
 
-    private static ObjectNode createBenchmarkNode(@Nonnull final ObjectNode syncNode,
-                                                  @Nonnull final String benchmark) {
-        final ObjectNode newBenchmarkNode = JsonNodeFactory.instance.objectNode();
-        newBenchmarkNode.set(EXECUTION_TIMES, JsonNodeFactory.instance.arrayNode());
-        newBenchmarkNode.set(AVERAGE, JsonNodeFactory.instance.numberNode(0));
-        newBenchmarkNode.set(DIFF, JsonNodeFactory.instance.numberNode(0));
+        final ObjectNode newSyncNode = JsonNodeFactory.instance.objectNode();
+        newSyncNode.set(CREATES_ONLY, createBenchmarkNode());
+        newSyncNode.set(UPDATES_ONLY, createBenchmarkNode());
+        newSyncNode.set(CREATES_AND_UPDATES, createBenchmarkNode());
 
-        syncNode.set(benchmark, newBenchmarkNode);
-        return syncNode;
+        return newSyncNode;
     }
 
-    static double calculateDiff(@Nonnull final String version,
-                                @Nonnull final String sync,
-                                @Nonnull final String benchmark,
-                                final double average) throws IOException {
-        final JsonNode rootNode = new ObjectMapper().readTree(getFileContent(BENCHMARK_RESULTS_FILE_PATH));
-        return calculateDiff(rootNode, version, sync, benchmark, average);
-    }
+    @Nonnull
+    private static ObjectNode createBenchmarkNode() {
 
-    private static double calculateDiff(@Nonnull final JsonNode originalRoot,
-                                @Nonnull final String version,
-                                @Nonnull final String sync,
-                                @Nonnull final String benchmark,
-                                final double average) {
-        return getLatestVersionName(originalRoot, version)
-            .map(latestVersionName -> originalRoot.get(latestVersionName)
-                                                  .get(sync)
-                                                  .get(benchmark)
-                                                  .get(AVERAGE))
-            .map(latestAverageNode -> average - latestAverageNode.asDouble())
-            // if there is no latest version (meaning this is the first benchmark for this module)
-            // then return a 0 diff.
-            .orElse(0.0);
-    }
+        final ObjectNode newBenchmarkNode = JsonNodeFactory.instance.objectNode();
+        newBenchmarkNode.set(EXECUTION_TIME, JsonNodeFactory.instance.numberNode(0));
 
-    private static Optional<String> getLatestVersionName(@Nonnull final JsonNode originalRoot,
-                                                         @Nonnull final String currentVersionName) {
-        return toStream(originalRoot.fieldNames()).reduce((firstVersion, secondVersion) ->
-            !currentVersionName.equals(secondVersion) ? secondVersion : firstVersion);
+        return newBenchmarkNode;
     }
 
-
-    private static String getFileContent(@Nonnull final String path) throws IOException {
-        final byte[] fileBytes = Files.readAllBytes(Paths.get(path));
-        return new String(fileBytes, UTF8_CHARSET);
+    private static void writeToFile(@Nonnull final String content) throws IOException {
+        Files.write(Paths.get(BENCHMARK_RESULTS_FILE_PATH), content.getBytes(UTF8_CHARSET));
     }
 
-    private static void writeToFile(@Nonnull final String content, @Nonnull final String path) throws IOException {
-        Files.write(Paths.get(path), content.getBytes(UTF8_CHARSET));
+    private BenchmarkUtils() {
     }
 }

src/benchmark/java/com/commercetools/sync/benchmark/CategorySyncBenchmark.java

@@ -1,6 +1,5 @@
 package com.commercetools.sync.benchmark;
 
-import com.commercetools.sync.commons.utils.SyncSolutionInfo;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -17,19 +16,19 @@ public class CategorySyncBenchmark {
     @Test
     public void sync_NewCategories_ShouldCreateCategories() throws IOException {
         // TODO: SHOULD BE IMPLEMENTED.
-        saveNewResult(SyncSolutionInfo.LIB_VERSION, CATEGORY_SYNC, CREATES_ONLY, 20000);
+        saveNewResult(CATEGORY_SYNC, CREATES_ONLY, 20000);
     }
 
     @Test
     public void sync_ExistingCategories_ShouldUpdateCategories() throws IOException {
         // TODO: SHOULD BE IMPLEMENTED.
-        saveNewResult(SyncSolutionInfo.LIB_VERSION, CATEGORY_SYNC, UPDATES_ONLY, 10000);
+        saveNewResult(CATEGORY_SYNC, UPDATES_ONLY, 10000);
     }
 
     @Test
     public void sync_WithSomeExistingCategories_ShouldSyncCategories() throws IOException {
         // TODO: SHOULD BE IMPLEMENTED.
-        saveNewResult(SyncSolutionInfo.LIB_VERSION, CATEGORY_SYNC, CREATES_AND_UPDATES, 30000);
+        saveNewResult(CATEGORY_SYNC, CREATES_AND_UPDATES, 30000);
     }
 
 }

src/benchmark/java/com/commercetools/sync/benchmark/InventorySyncBenchmark.java

@@ -1,6 +1,5 @@
 package com.commercetools.sync.benchmark;
 
-import com.commercetools.sync.commons.utils.SyncSolutionInfo;
 import com.commercetools.sync.inventories.InventorySync;
 import com.commercetools.sync.inventories.InventorySyncOptions;
 import com.commercetools.sync.inventories.InventorySyncOptionsBuilder;
@@ -26,9 +25,8 @@
 import static com.commercetools.sync.benchmark.BenchmarkUtils.CREATES_ONLY;
 import static com.commercetools.sync.benchmark.BenchmarkUtils.INVENTORY_SYNC;
 import static com.commercetools.sync.benchmark.BenchmarkUtils.NUMBER_OF_RESOURCE_UNDER_TEST;
-import static com.commercetools.sync.benchmark.BenchmarkUtils.THRESHOLD;
+import static com.commercetools.sync.benchmark.BenchmarkUtils.THRESHOLD_EXCEEDED_ERROR;
 import static com.commercetools.sync.benchmark.BenchmarkUtils.UPDATES_ONLY;
-import static com.commercetools.sync.benchmark.BenchmarkUtils.calculateDiff;
 import static com.commercetools.sync.benchmark.BenchmarkUtils.saveNewResult;
 import static com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics.assertThat;
 import static com.commercetools.sync.integration.commons.utils.ChannelITUtils.deleteChannels;
@@ -57,25 +55,23 @@ public static void tearDown() {
 
     @Test
     public void sync_NewInventories_ShouldCreateInventories() throws IOException {
+        // preparation
         final List<InventoryEntryDraft> inventoryEntryDrafts = buildInventoryDrafts(NUMBER_OF_RESOURCE_UNDER_TEST);
-
-
         final InventorySyncOptions inventorySyncOptions = InventorySyncOptionsBuilder.of(CTP_TARGET_CLIENT)
                                                                                      .build();
         final InventorySync inventorySync = new InventorySync(inventorySyncOptions);
 
-
+        // benchmark
         final long beforeSync = System.currentTimeMillis();
         final InventorySyncStatistics inventorySyncStatistics =
             executeBlocking(inventorySync.sync(inventoryEntryDrafts));
         final long totalTime = System.currentTimeMillis() - beforeSync;
 
 
-        // Caclulate sync time and assert on threshold
-        final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, INVENTORY_SYNC, CREATES_ONLY, totalTime);
-        assertThat(diff).withFailMessage(format("Diff of benchmark '%e' is longer than expected"
-                            + " threshold of '%d'.", diff, THRESHOLD))
-                        .isLessThanOrEqualTo(THRESHOLD);
+        // assert on threshold (based on history of benchmarks; highest was ~9 seconds)
+        final int threshold = 18000; // double of the highest benchmark
+        assertThat(totalTime).withFailMessage(format(THRESHOLD_EXCEEDED_ERROR, totalTime, threshold))
+                             .isLessThan(threshold);
 
         // Assert actual state of CTP project (total number of existing inventories)
         final CompletableFuture<Integer> totalNumberOfInventories =
@@ -92,21 +88,21 @@ public void sync_NewInventories_ShouldCreateInventories() throws IOException {
         assertThat(inventorySyncStatistics)
             .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, NUMBER_OF_RESOURCE_UNDER_TEST, 0, 0);
 
-        saveNewResult(SyncSolutionInfo.LIB_VERSION, INVENTORY_SYNC, CREATES_ONLY, totalTime);
+        saveNewResult(INVENTORY_SYNC, CREATES_ONLY, totalTime);
     }
 
     @Ignore
     @Test
     public void sync_ExistingInventories_ShouldUpdateInventories() throws IOException {
         // TODO: SHOULD BE IMPLEMENTED.
-        saveNewResult(SyncSolutionInfo.LIB_VERSION, INVENTORY_SYNC, UPDATES_ONLY, 50000);
+        saveNewResult(INVENTORY_SYNC, UPDATES_ONLY, 50000);
     }
 
     @Ignore
     @Test
     public void sync_WithSomeExistingInventories_ShouldSyncInventories() throws IOException {
         // TODO: SHOULD BE IMPLEMENTED.
-        saveNewResult(SyncSolutionInfo.LIB_VERSION, INVENTORY_SYNC, CREATES_AND_UPDATES, 30000);
+        saveNewResult(INVENTORY_SYNC, CREATES_AND_UPDATES, 30000);
     }
 
     @Nonnull