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 <[email protected]> Authored: Tue Nov 22 19:33:26 2016 +0100 Committer: Stephan Erb <[email protected]> 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
