You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2015/04/23 02:24:51 UTC
Review Request 33458: Implementing PendingTaskProcessor benchmark.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/
-----------------------------------------------------------
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
Adjusting preemptor benchmark to test the correct functionality.
```
Benchmark (numPendingTasks) Mode Cnt Score Error Units
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
```
Diffs
-----
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 12f42a92a28c68fcc55bf2f0b130ffed8c4d4879
src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
Diff: https://reviews.apache.org/r/33458/diff/
Testing
-------
./gradlew jmh
Thanks,
Maxim Khutornenko
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81850
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Farner
On April 28, 2015, 5:24 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33458/
> -----------------------------------------------------------
>
> (Updated April 28, 2015, 5:24 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adjusting preemptor benchmark to test the correct functionality.
>
> ```
> Benchmark (numPendingTasks) Mode Cnt Score Error Units
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 372addc6f6288ebf7c3dbf1e26902c2b62b69669
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
>
> Diff: https://reviews.apache.org/r/33458/diff/
>
>
> Testing
> -------
>
> ./gradlew jmh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81843
-----------------------------------------------------------
Master (e9d723d) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 28, 2015, 5:24 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33458/
> -----------------------------------------------------------
>
> (Updated April 28, 2015, 5:24 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adjusting preemptor benchmark to test the correct functionality.
>
> ```
> Benchmark (numPendingTasks) Mode Cnt Score Error Units
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 372addc6f6288ebf7c3dbf1e26902c2b62b69669
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
>
> Diff: https://reviews.apache.org/r/33458/diff/
>
>
> Testing
> -------
>
> ./gradlew jmh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/
-----------------------------------------------------------
(Updated April 28, 2015, 5:24 p.m.)
Review request for Aurora and Bill Farner.
Changes
-------
Bill's comment.
Repository: aurora
Description
-------
Adjusting preemptor benchmark to test the correct functionality.
```
Benchmark (numPendingTasks) Mode Cnt Score Error Units
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
```
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 372addc6f6288ebf7c3dbf1e26902c2b62b69669
src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
Diff: https://reviews.apache.org/r/33458/diff/
Testing
-------
./gradlew jmh
Thanks,
Maxim Khutornenko
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Bill Farner <wf...@apache.org>.
> On April 28, 2015, 12:37 a.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 243
> > <https://reviews.apache.org/r/33458/diff/1/?file=940120#file940120line243>
> >
> > I'm not too fond of this, it points out that `BenchmarkSettings` is being used for distinct purposes, and may become difficult to reason about. Can you accomplish this by saving the tasks from `PreemptorSlotSearchBenchmark` directly, since it's the oddball here?
>
> Maxim Khutornenko wrote:
> I actually like how BenchmarkSettings.getTasks() fits the "count" supported by the Tasks.Builder. There is no reason to restrict benchmark runs to just a single test task. How about generalizing this to support multiple tasks instead? E.g.:
> ```
> @Benchmark
> public boolean runBenchmark() {
> boolean result = false;
> for (IScheduledTask task : settings.getTasks()) {
> result = taskScheduler.schedule(task.getAssignedTask().getTaskId());
> }
> return result;
> }
> ```
This crossed my mind as well, i would indeed prefer that.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81759
-----------------------------------------------------------
On April 23, 2015, 12:24 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33458/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 12:24 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adjusting preemptor benchmark to test the correct functionality.
>
> ```
> Benchmark (numPendingTasks) Mode Cnt Score Error Units
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 12f42a92a28c68fcc55bf2f0b130ffed8c4d4879
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
>
> Diff: https://reviews.apache.org/r/33458/diff/
>
>
> Testing
> -------
>
> ./gradlew jmh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On April 28, 2015, 12:37 a.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 243
> > <https://reviews.apache.org/r/33458/diff/1/?file=940120#file940120line243>
> >
> > I'm not too fond of this, it points out that `BenchmarkSettings` is being used for distinct purposes, and may become difficult to reason about. Can you accomplish this by saving the tasks from `PreemptorSlotSearchBenchmark` directly, since it's the oddball here?
I actually like how BenchmarkSettings.getTasks() fits the "count" supported by the Tasks.Builder. There is no reason to restrict benchmark runs to just a single test task. How about generalizing this to support multiple tasks instead? E.g.:
```
@Benchmark
public boolean runBenchmark() {
boolean result = false;
for (IScheduledTask task : settings.getTasks()) {
result = taskScheduler.schedule(task.getAssignedTask().getTaskId());
}
return result;
}
```
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81759
-----------------------------------------------------------
On April 23, 2015, 12:24 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33458/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 12:24 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adjusting preemptor benchmark to test the correct functionality.
>
> ```
> Benchmark (numPendingTasks) Mode Cnt Score Error Units
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 12f42a92a28c68fcc55bf2f0b130ffed8c4d4879
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
>
> Diff: https://reviews.apache.org/r/33458/diff/
>
>
> Testing
> -------
>
> ./gradlew jmh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81759
-----------------------------------------------------------
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
<https://reviews.apache.org/r/33458/#comment132221>
I'm not too fond of this, it points out that `BenchmarkSettings` is being used for distinct purposes, and may become difficult to reason about. Can you accomplish this by saving the tasks from `PreemptorSlotSearchBenchmark` directly, since it's the oddball here?
- Bill Farner
On April 23, 2015, 12:24 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33458/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 12:24 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adjusting preemptor benchmark to test the correct functionality.
>
> ```
> Benchmark (numPendingTasks) Mode Cnt Score Error Units
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 12f42a92a28c68fcc55bf2f0b130ffed8c4d4879
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
>
> Diff: https://reviews.apache.org/r/33458/diff/
>
>
> Testing
> -------
>
> ./gradlew jmh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81255
-----------------------------------------------------------
Master (352e0ef) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 23, 2015, 12:24 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33458/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 12:24 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adjusting preemptor benchmark to test the correct functionality.
>
> ```
> Benchmark (numPendingTasks) Mode Cnt Score Error Units
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s
> SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 12f42a92a28c68fcc55bf2f0b130ffed8c4d4879
> src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a
>
> Diff: https://reviews.apache.org/r/33458/diff/
>
>
> Testing
> -------
>
> ./gradlew jmh
>
>
> Thanks,
>
> Maxim Khutornenko
>
>