You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by se...@apache.org on 2016/11/22 18:38:56 UTC

aurora git commit: Fix performance regression in AttributeAggregate performance.

Repository: aurora
Updated Branches:
  refs/heads/master 99164834a -> 320ee0810


Fix performance regression in AttributeAggregate performance.

This commit ensures AttributeAggregate will only be computed if needed by
limit constraints. This is the case in 0.16 but broken on master since the
introduction of scheduling attempts with multiple tasks.

In order to better model the latter this patch also updates the the
benchmarks to schedule multipe tasks per scheduleTask call.

Without the fix:
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  thrpt   10  404.446 � 31.252  ops/s
SchedulingBenchmarks.FillClusterBenchmark.runBenchmark  thrpt   10  7.233 � 3.058  ops/s

With the fix:
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark  thrpt   10  432.245 � 16.963  ops/s
SchedulingBenchmarks.FillClusterBenchmark.runBenchmark  thrpt   10  87.560 � 14.600  ops/s

Bugs closed: AURORA-1802

Reviewed at https://reviews.apache.org/r/53918/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/320ee081
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/320ee081
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/320ee081

Branch: refs/heads/master
Commit: 320ee0810e95c79e2e3d31c9af041de7489c9f22
Parents: 9916483
Author: Stephan Erb <se...@apache.org>
Authored: Tue Nov 22 19:33:26 2016 +0100
Committer: Stephan Erb <se...@apache.org>
Committed: Tue Nov 22 19:33:26 2016 +0100

----------------------------------------------------------------------
 .../aurora/benchmark/SchedulingBenchmarks.java  | 13 ++++++--
 .../scheduler/filter/AttributeAggregate.java    | 34 ++++++++++++++++----
 .../scheduler/storage/db/AttributeMapper.xml    |  1 -
 3 files changed, 38 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/320ee081/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
----------------------------------------------------------------------
diff --git a/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java b/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
index 631b2cf..fa37236 100644
--- a/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
+++ b/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
@@ -13,12 +13,15 @@
  */
 package org.apache.aurora.benchmark;
 
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import javax.inject.Singleton;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.common.eventbus.EventBus;
 import com.google.inject.AbstractModule;
@@ -228,10 +231,14 @@ public class SchedulingBenchmarks {
     protected Set<String> schedule(Set<IScheduledTask> tasks) {
       return storage.write((Storage.MutateWork.Quiet<Set<String>>) store -> {
         Set<String> result = null;
-        for (IScheduledTask task : tasks) {
+
+        List<List<IScheduledTask>> partitionedTasks = Lists.newArrayList(
+            Iterators.partition(tasks.iterator(), 5));
+
+        for (List<IScheduledTask> partition : partitionedTasks) {
           result = taskScheduler.schedule(
               store,
-              ImmutableSet.of(task.getAssignedTask().getTaskId()));
+              org.apache.aurora.scheduler.base.Tasks.ids(partition));
         }
         return result;
       });
@@ -248,7 +255,7 @@ public class SchedulingBenchmarks {
       return new BenchmarkSettings.Builder()
           .setSiblingClusterUtilization(0.01)
           .setVictimClusterUtilization(0.01)
-          .setHostAttributes(new Hosts.Builder().setNumHostsPerRack(2).build(100000))
+          .setHostAttributes(new Hosts.Builder().setNumHostsPerRack(2).build(200000))
           .setTasks(new Tasks.Builder().build(0))
           .build();
     }

http://git-wip-us.apache.org/repos/asf/aurora/blob/320ee081/src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java b/src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
index f04149e..60f141d 100644
--- a/src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
+++ b/src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
@@ -49,8 +49,19 @@ public final class AttributeAggregate {
    */
   private Supplier<Multiset<Pair<String, String>>> aggregate;
 
+  private boolean isInitialized = false;
+
   private AttributeAggregate(Supplier<Multiset<Pair<String, String>>> aggregate) {
-    this.aggregate = Suppliers.memoize(aggregate);
+    this.aggregate = Suppliers.memoize(
+        () -> {
+          initialize();
+          return aggregate.get();
+        }
+    );
+  }
+
+  private void initialize() {
+    isInitialized = true; // inlining this assignment yields a PMD false positive
   }
 
   /**
@@ -99,7 +110,7 @@ public final class AttributeAggregate {
     return new AttributeAggregate(aggregator);
   }
 
-  static ImmutableMultiset.Builder<Pair<String, String>> addAttributes(
+  private static ImmutableMultiset.Builder<Pair<String, String>> addAttributes(
       ImmutableMultiset.Builder<Pair<String, String>> builder,
       Iterable<IAttribute> attributes) {
 
@@ -112,10 +123,21 @@ public final class AttributeAggregate {
   }
 
   public void updateAttributeAggregate(IHostAttributes attributes) {
-    ImmutableMultiset.Builder<Pair<String, String>> builder = new ImmutableMultiset.Builder<>();
-    builder.addAll(aggregate.get());
-    addAttributes(builder, attributes.getAttributes());
-    aggregate = Suppliers.memoize(() -> builder.build());
+    // If the aggregate supplier has not been populated there is no need to update it here.
+    // All tasks attributes will be picked up by the wrapped task query if executed at a
+    // later point in time.
+    if (isInitialized) {
+      final Supplier<Multiset<Pair<String, String>>> previous = aggregate;
+      aggregate = Suppliers.memoize(
+          () -> {
+            ImmutableMultiset.Builder<Pair<String, String>> builder
+                = new ImmutableMultiset.Builder<>();
+            builder.addAll(previous.get());
+            addAttributes(builder, attributes.getAttributes());
+            return builder.build();
+          }
+      );
+    }
   }
 
   @VisibleForTesting

http://git-wip-us.apache.org/repos/asf/aurora/blob/320ee081/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
----------------------------------------------------------------------
diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml b/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
index 91c76ca..d49c90b 100644
--- a/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
+++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
@@ -67,7 +67,6 @@
       a.host AS a_host,
       a.mode AS a_mode,
       a.slave_id AS a_slave_id,
-      a.slave_id AS v_slave_id,
       v.id AS v_id,
       v.name AS v_name,
       v.value AS v_value