-
Notifications
You must be signed in to change notification settings - Fork 7.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.x: Upgrade to Gradle 4.3.1, add TakeUntilPerf #6029
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.x #6029 +/- ##
============================================
- Coverage 98.32% 98.27% -0.05%
- Complexity 6171 6176 +5
============================================
Files 665 665
Lines 44726 44726
Branches 6206 6206
============================================
- Hits 43975 43956 -19
- Misses 219 232 +13
- Partials 532 538 +6
Continue to review full report at Codecov.
|
Holy moly |
} | ||
}); | ||
|
||
while (cdl.getCount() != 0) { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akarnokd I wonder if this loop affects benchmark in positive or negative way, it consumes single core to max
Should cdl.await()
be used instead? it parks the thread
I see this pattern in few other benchmarks but idk if it's intentional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// It shouldn't affect the comparison though since loop is present in both Flowable and Observable benchmarks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem await
is that on my machines they can't wake up that fast and the throughput usually caps out around 200k ops/s, even if the underlying work finished much faster.
For sorter sequences, typically in the range of 1000-10000 items, a spin-wait will result in a much higher throughput number as the end-of-work is detected more eagerly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhhh damn, I think you've mentioned that problem before
What if JMH would be event/callback based so we could benchmark async code like RxJava in a reactive manner?
ie:
@Benchmark
public void flowable(BenchmarkObserver benchmarkObserver) {
flowable.subscribe(this, Functions.emptyConsumer(), new Action() {
@Override
public void run() throws Exception {
benchmarkObserver.onComplete();
}
});
}
The JMH plugin 0.4.4 has some shortcomings that prevent the execution of unit tests on Windows 10 (and on some very restricted Linuxes). Version 0.4.5 has been fixed in this regard but it also requires Gradle 4.3.x. Unfortunately, there are no newer versions to the JMH plugin so this is likely as far as we can go with versions.
I've also added a new benchmark to measure the overhead in
takeUntil
. Here are the results:i7 4790, Windows 10 x64, Java 8u172, JMH 1.20:
The
observable
here uses an older algorithms & structure and is generally relaying half the items under the same time amonut than theflowable
version. PR #6028 can then be evaluated with this benchmark.