----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33106/#review79963 -----------------------------------------------------------
The way I read these benchmark results peformance drops for all but two use cases by 20-70%. That's quite a big hit to our baseline with unclear motivation. I don't necessarily find the complexity reduction gain worthwhile the perf hit. What drives your changes here? Is there a connection to other work that would justify this refactoring? - Maxim Khutornenko On April 12, 2015, 2:11 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33106/ > ----------------------------------------------------------- > > (Updated April 12, 2015, 2:11 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > This removes the Supplier/memoize behavior in `AttributeAggregate`, > performing all aggregation up front. > > > Diffs > ----- > > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > ce87344dc18818faa7a1a0298143dcaaaa81fff7 > src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java > ed82ae99f23d5a7f1634261205cbe5339fe4cec8 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java > c5643d9d99fc46d55fd6c48161230139fb7f12b8 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java > 88c0163170ebc25995d9ef8b1543335a4322bb8e > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java > bcd1b4e854f5ea227268c73156bc97c7c937c1de > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java > 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java > 7e2d1c54362b33cc3507a4bc3e3ccc02ca29bd6f > > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java > b80e558f18b817814e4768b13ff94aa816d28543 > > src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java > 61cea326ababcd6242a3c5a6dcf8d0b3ca7fbdd6 > > src/test/java/org/apache/aurora/scheduler/filter/AttributeAggregateTest.java > 4b565769d79862326efcb31be694f95f333c89c6 > > src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java > d06b89cc319aa7b4479124cbb2cb224cdb662e05 > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java > aca0234e037e85202d182affa2c0e988c6cfc854 > > Diff: https://reviews.apache.org/r/33106/diff/ > > > Testing > ------- > > See diff of relevant benchmark sections below (top section is master, bottom > is with this patch). > > As should be expected, performance declines in two scenarios that did not > 'pull' the `Supplier`: > - there are no resource offers > - the task only has value constraints (therefore unaffected by state of the > rest of the job) > > I argue that in practice, these scenarios (no offers, value-only constraints) > are rare enough to not be worth the code complexity. > > ``` > < SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark > N/A N/A thrpt 10 593015.942 ± 5782.045 ops/s > < SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > N/A N/A thrpt 10 44781.172 ± 306.152 ops/s > < > SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark > N/A N/A thrpt 10 3397.620 ± 35.340 ops/s > < SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark > N/A N/A thrpt 10 240.376 ± 6.679 ops/s > < > SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark > N/A N/A thrpt 10 44435.419 ± 291.268 ops/s > --- > > SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark > > N/A N/A thrpt 10 181543.224 ± 2313.677 > > ops/s > > SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > > N/A N/A thrpt 10 35623.691 ± 303.796 > > ops/s > > SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark > > N/A N/A thrpt 10 3318.727 ± 19.131 > > ops/s > > SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark > > N/A N/A thrpt 10 235.248 ± 5.928 > > ops/s > > SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark > > N/A N/A thrpt 10 35468.747 ± 293.517 > > ops/s > ``` > > > Thanks, > > Bill Farner > >