> On Feb. 5, 2015, 12:01 a.m., Bill Farner wrote: > > Can you see any opportunity to break this diff apart? As it stands i'm > > having a hard time giving a thoughtful review. Perhaps you can start by > > introducing the `Assignment` class? > > Maxim Khutornenko wrote: > I'd really prefer keeping this diff as a whole. The Assignment class > would not make sense without the entire picture in mind.
It's okay if you commit them back-to-back, but we really need to exercise a pattern for incrementally building features even if they appear as code islands in the commit history. Doing so makes it easier to review with confidence, and more likely that you get faster review turnaround. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28617/#review71079 ----------------------------------------------------------- On Feb. 4, 2015, 11:38 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28617/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 11:38 p.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-909 > https://issues.apache.org/jira/browse/AURORA-909 > > > Repository: aurora > > > Description > ------- > > Modified the task offer/task matching logic to skip offer matching for tasks > previously vetoed statically. > > Real life testing in vagrant (see pictures) shows close to 50% improvement in > task scheduling performance. > > Testing with JMH shows over 97% better perf when testing with disabled > preemptor (1 scheduling loop): > ``` > Master > Benchmark > Mode Samples Score Error Units > o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark > avgt 100 8291046.074 ± 145251.995 ns/op > o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > avgt 100 7522269.050 ± 142446.265 ns/op > > This RB > Benchmark > Mode Samples Score Error Units > o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark > avgt 100 204171.046 ± 3800.124 ns/op > o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > avgt 100 215854.129 ± 8959.851 ns/op > ``` > > Testing with preemptor enabled and no tasks eligible for preemption gives > around 40% improvement (2 scheduling loops): > ``` > Master > Benchmark > Mode Samples Score Error Units > o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark > avgt 100 1767479.299 ± 26907.571 ns/op > o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > avgt 100 1538682.287 ± 119119.911 ns/op > > This RB > Benchmark > Mode Samples Score Error Units > o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark > avgt 100 1105731.141 ± 10040.721 ns/op > o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > avgt 100 939230.662 ± 11091.505 ns/op > ``` > > Testing with preemptor enabled and running the worst case possible scenario > (every slave is eligible and all tasks are victims) yields the least > improvement 2-3% (3 scheduling loops). > ``` > Master > Benchmark > Mode Samples Score Error Units > o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark > avgt 100 11043701.243 ± 40550.259 ns/op > o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > avgt 100 10478631.055 ± 178833.158 ns/op > o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark > avgt 100 116258653.000 ± 403080.017 ns/op > > This RB > Benchmark > Mode Samples Score Error Units > o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark > avgt 100 10886116.889 ± 193934.324 ns/op > o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > avgt 100 10182572.955 ± 35740.891 ns/op > o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark > avgt 100 113656994.000 ± 424163.759 ns/op > ``` > > > Diffs > ----- > > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 8c11ef8bd6609f3e4d97ca154d922898f8362446 > src/jmh/java/org/apache/aurora/benchmark/Tasks.java > 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 > src/main/java/org/apache/aurora/scheduler/TaskVars.java > f017cdd26ca40138a7e141f21613ed567314c399 > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java > f66383830140e5eaba436f35ebb5192eee65947a > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java > ce47ff152e303fd2116bc3b9e91c0c1a8f76f258 > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java > 6a43bcd1719e8aa32fd3fcb7387d0318c3c0b804 > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java > f06fdaeb92e154d0982bdabed5df93e7bcba9048 > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java > e1c29747c9854cf75bf63f6f085cf40ca68989af > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > 4e7efb3c1214c3d193afd61f162713490eb8effb > src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java > 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java > 5647349854a5e04de749c4d809684a0066d4da06 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java > 6cc13231560996b144101eba36577f49017aba06 > > src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java > 52ee7c1e3742d9315c7e7aaa77677121e1e9288d > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java > 411a55a8d85f60bb2703468f2d69b64b2736eee4 > > Diff: https://reviews.apache.org/r/28617/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > Tested in vagrant > > > File Attachments > ---------------- > > NoStaticVetoFiltering.png > > https://reviews.apache.org/media/uploaded/files/2014/12/03/7945c60b-4135-4016-a9bf-8d4815a4a573__NoStaticVetoFiltering.png > StaticVetoFiltering.png > > https://reviews.apache.org/media/uploaded/files/2014/12/03/2f73b94a-5ba9-43b6-922e-e9e4ec18d0bb__StaticVetoFiltering.png > > > Thanks, > > Maxim Khutornenko > >
