Re: Review Request 31508: Removing redundant scheduling loop in preemptor.

2015-02-27 Thread Bill Farner

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31508/#review74514
---

Ship it!


Context is that the removed code was attempting to mitigate a race where we 
preempt a task unnecessarily because new offers have arrived.  Given that 
preemption begins immediately after we have passed over the offers, this 
additional pass is redundant.  Additionally, the race still exists (an offer 
could arrive _right after_ we preempt), so the mitigation is of dubious value.

- Bill Farner


On Feb. 27, 2015, 12:04 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31508/
 ---
 
 (Updated Feb. 27, 2015, 12:04 a.m.)
 
 
 Review request for Aurora, Bill Farner and Zameer Manji.
 
 
 Bugs: AURORA-1156
 https://issues.apache.org/jira/browse/AURORA-1156
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This is #1 from the attached ticket. Brings anywhere between 2% and 18% 
 better perf in bechmark scenarios.
 
 BEFORE:
 ```
 Benchmark 
   Mode  Samples Score Error  Units
 o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
   avgt  100909677.646 ±   10103.466  ns/op
 o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100   1332768.205 ±   16664.386  ns/op
 o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
  avgt  100  69304405.590 ± 1536571.317  ns/op
 o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100870348.707 ±   16815.495  ns/op
 ```
 
 AFTER:
 ```
 Benchmark 
   Mode  Samples Score Error  Units
 o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
   avgt  100749864.522 ±6568.372  ns/op
 o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100   1125995.085 ±   19241.796  ns/op
 o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
  avgt  100  68028627.539 ± 1412569.919  ns/op
 o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100766747.584 ±   13310.142  ns/op
 ```
 
 
 Diffs
 -
 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 a4e8dd3b9b58dac2d8507042500b7a479d46ebc0 
   
 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
  44cd8f79493f0f247cd876ef78b30b4f813314c4 
 
 Diff: https://reviews.apache.org/r/31508/diff/
 
 
 Testing
 ---
 
 ./gradlew jmh
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 31508: Removing redundant scheduling loop in preemptor.

2015-02-27 Thread Maxim Khutornenko

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31508/
---

(Updated Feb. 27, 2015, 8:23 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Rebased.


Bugs: AURORA-1156
https://issues.apache.org/jira/browse/AURORA-1156


Repository: aurora


Description
---

This is #1 from the attached ticket. Brings anywhere between 2% and 18% better 
perf in bechmark scenarios.

BEFORE:
```
Benchmark   
Mode  Samples Score Error  Units
o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
  avgt  100909677.646 ±   10103.466  ns/op
o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
avgt  100   1332768.205 ±   16664.386  ns/op
o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
 avgt  100  69304405.590 ± 1536571.317  ns/op
o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
avgt  100870348.707 ±   16815.495  ns/op
```

AFTER:
```
Benchmark   
Mode  Samples Score Error  Units
o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
  avgt  100749864.522 ±6568.372  ns/op
o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
avgt  100   1125995.085 ±   19241.796  ns/op
o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
 avgt  100  68028627.539 ± 1412569.919  ns/op
o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
avgt  100766747.584 ±   13310.142  ns/op
```


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
833a3e0b088780e21f5f16434327c96447a25115 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
 2845b3f72fca5c329a8b81ce796370ad95d94f02 

Diff: https://reviews.apache.org/r/31508/diff/


Testing
---

./gradlew jmh


Thanks,

Maxim Khutornenko



Re: Review Request 31508: Removing redundant scheduling loop in preemptor.

2015-02-26 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31508/#review74409
---

Ship it!


Master (766d1c9) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On Feb. 27, 2015, 12:04 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31508/
 ---
 
 (Updated Feb. 27, 2015, 12:04 a.m.)
 
 
 Review request for Aurora, Bill Farner and Zameer Manji.
 
 
 Bugs: AURORA-1156
 https://issues.apache.org/jira/browse/AURORA-1156
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This is #1 from the attached ticket. Brings anywhere between 2% and 18% 
 better perf in bechmark scenarios.
 
 BEFORE:
 ```
 Benchmark 
   Mode  Samples Score Error  Units
 o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
   avgt  100909677.646 ±   10103.466  ns/op
 o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100   1332768.205 ±   16664.386  ns/op
 o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
  avgt  100  69304405.590 ± 1536571.317  ns/op
 o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100870348.707 ±   16815.495  ns/op
 ```
 
 AFTER:
 ```
 Benchmark 
   Mode  Samples Score Error  Units
 o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
   avgt  100749864.522 ±6568.372  ns/op
 o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100   1125995.085 ±   19241.796  ns/op
 o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark
  avgt  100  68028627.539 ± 1412569.919  ns/op
 o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
 avgt  100766747.584 ±   13310.142  ns/op
 ```
 
 
 Diffs
 -
 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 a4e8dd3b9b58dac2d8507042500b7a479d46ebc0 
   
 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
  44cd8f79493f0f247cd876ef78b30b4f813314c4 
 
 Diff: https://reviews.apache.org/r/31508/diff/
 
 
 Testing
 ---
 
 ./gradlew jmh
 
 
 Thanks,
 
 Maxim Khutornenko