Address PR feedback.

Signed-off-by: Yury-Fridlyand <[email protected]>
valkey-io · Dec 5, 2023 · d3f707f · d3f707f
1 parent 199ed5a
commit d3f707f
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 35 deletions.
diff --git a/java/benchmarks/src/main/java/babushka/benchmarks/BenchmarkingApp.java b/java/benchmarks/src/main/java/babushka/benchmarks/BenchmarkingApp.java
@@ -41,8 +41,8 @@ public static void main(String[] args) {
     for (ClientName client : runConfiguration.clients) {
       switch (client) {
         case JEDIS:
-          // run testClientSetGet on JEDIS pseudo-sync client
-          System.out.println("Run JEDIS async client");
+          // run testClientSetGet on JEDIS sync client
+          System.out.println("Run JEDIS sync client");
           break;
         case LETTUCE:
           // run testClientSetGet on LETTUCE async client
@@ -75,12 +75,7 @@ private static Options getOptions() {
             .desc("Number of concurrent tasks [100, 1000]")
             .build());
     options.addOption(
-        Option.builder("clients")
-            .hasArg(true)
-            .desc(
-                "one of: all|jedis|jedis_async|lettuce|lettuce_async|"
-                    + "babushka|babushka_async|all_async|all_sync")
-            .build());
+        Option.builder("clients").hasArg(true).desc("one of: all|jedis|lettuce|babushka").build());
     options.addOption(Option.builder("host").hasArg(true).desc("Hostname [localhost]").build());
     options.addOption(Option.builder("port").hasArg(true).desc("Port number [6379]").build());
     options.addOption(
@@ -178,7 +173,7 @@ private static int[] parseIntListOption(String line) throws ParseException {
   }
 
   public enum ClientName {
-    JEDIS("Jedis"), // async
+    JEDIS("Jedis"), // sync
     LETTUCE("Lettuce"), // async
     BABUSHKA("Babushka"), // async
     ALL("All");

diff --git a/java/benchmarks/src/main/java/babushka/benchmarks/clients/AsyncClient.java b/java/benchmarks/src/main/java/babushka/benchmarks/clients/AsyncClient.java
@@ -21,8 +21,8 @@ default <T> T waitForResult(Future<T> future) {
   default <T> T waitForResult(Future<T> future, long timeout) {
     try {
       return future.get(timeout, TimeUnit.MILLISECONDS);
-    } catch (TimeoutException ignored) {
-      return null;
+    } catch (TimeoutException e) {
+      throw new RuntimeException("A task timed out", e);
     } catch (ExecutionException e) {
       throw new RuntimeException("Client error", e);
     } catch (InterruptedException e) {

diff --git a/java/benchmarks/src/main/java/babushka/benchmarks/utils/Benchmarking.java b/java/benchmarks/src/main/java/babushka/benchmarks/utils/Benchmarking.java
@@ -12,7 +12,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Function;
 import java.util.function.Supplier;
 import org.apache.commons.lang3.tuple.Pair;
 
@@ -44,15 +43,15 @@ public static String generateKeySet() {
   }
 
   public interface Operation {
-    void go() throws InterruptedException, ExecutionException;
+    void go(Client client) throws InterruptedException, ExecutionException;
   }
 
   public static Pair<ChosenAction, Long> measurePerformance(
-      Client client, Map<ChosenAction, Function<Client, Operation>> actions) {
+      Client client, Map<ChosenAction, Operation> actions) {
     var action = randomAction();
     long before = System.nanoTime();
     try {
-      actions.get(action).apply(client).go();
+      actions.get(action).go(client);
     } catch (ExecutionException e) {
       throw new RuntimeException("Client error", e);
     } catch (InterruptedException e) {
@@ -223,15 +222,14 @@ private static CompletableFuture<Map<ChosenAction, ArrayList<Long>>> createTask(
                   ChosenAction.GET_NON_EXISTING, new ArrayList<Long>(),
                   ChosenAction.SET, new ArrayList<Long>());
           var actions = getActionMap(dataSize, async);
-          int iterationIncrement = iterationCounter.getAndIncrement();
-          int clientIndex = iterationIncrement % clients.size();
 
           if (debugLogging) {
-            System.out.printf(
-                "%n concurrent = %d/%d, client# = %d/%d%n",
-                taskNumDebugging, concurrentNum, clientIndex + 1, clientCount);
+            System.out.printf("%n concurrent = %d/%d%n", taskNumDebugging, concurrentNum);
           }
-          while (iterationIncrement < iterations) {
+          while (iterationCounter.get() < iterations) {
+            int iterationIncrement = iterationCounter.getAndIncrement();
+            int clientIndex = iterationIncrement % clients.size();
+
             if (debugLogging) {
               System.out.printf(
                   "> iteration = %d/%d, client# = %d/%d%n",
@@ -240,43 +238,34 @@ private static CompletableFuture<Map<ChosenAction, ArrayList<Long>>> createTask(
 
             // operate and calculate tik-tok
             Pair<ChosenAction, Long> result = measurePerformance(clients.get(clientIndex), actions);
-            if (result != null) {
-              taskActionResults.get(result.getLeft()).add(result.getRight());
-            }
-
-            iterationIncrement = iterationCounter.getAndIncrement();
-            clientIndex = iterationIncrement % clients.size();
+            taskActionResults.get(result.getLeft()).add(result.getRight());
           }
           return taskActionResults;
         });
   }
 
-  public static Map<ChosenAction, Function<Client, Operation>> getActionMap(
-      int dataSize, boolean async) {
+  public static Map<ChosenAction, Operation> getActionMap(int dataSize, boolean async) {
 
     String value = "0".repeat(dataSize);
     return Map.of(
         ChosenAction.GET_EXISTING,
-        client ->
-            () -> {
+            (client) -> {
               if (async) {
                 ((AsyncClient) client).asyncGet(generateKeySet()).get();
               } else {
                 ((SyncClient) client).get(generateKeySet());
               }
             },
         ChosenAction.GET_NON_EXISTING,
-        client ->
-            () -> {
+            (client) -> {
               if (async) {
                 ((AsyncClient) client).asyncGet(generateKeyGet()).get();
               } else {
                 ((SyncClient) client).get(generateKeyGet());
               }
             },
         ChosenAction.SET,
-        client ->
-            () -> {
+            (client) -> {
               if (async) {
                 ((AsyncClient) client).asyncSet(generateKeySet(), value).get();
               } else {