Skip to content
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: Improve Observable.takeUntil #6028

Merged
merged 1 commit into from
May 30, 2018

Conversation

akarnokd
Copy link
Member

This PR upgrades the Observable.takeUntil to a newer algorithm (the Flowable version is up-to-date).

Some unit test remnants from the v1 era were upgraded too as the new algorithm no longer disposes the source or other if they terminate on their own (the Reactive Streams specification doesn't allow that anyway).

@akarnokd akarnokd added this to the 2.2 milestone May 29, 2018
@akarnokd
Copy link
Member Author

@artem-zinnatullin
Copy link
Contributor

Algorithm of the operator itself seems unchanged, only event serialization and disposing is different right?

I assume HalfSerializer is faster than SerializedObserver because it's non-blocking, but if you happen to have benchmark results I'd be happy to check them out :)
(I might need something like this in Domic)

LGTM anyway 👍

@akarnokd
Copy link
Member Author

PR state is stuck, I'll close & reopen this.

@akarnokd akarnokd closed this May 30, 2018
@akarnokd akarnokd reopened this May 30, 2018
@codecov
Copy link

codecov bot commented May 30, 2018

Codecov Report

Merging #6028 into 2.x will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6028      +/-   ##
============================================
+ Coverage     98.26%   98.33%   +0.06%     
  Complexity     6173     6173              
============================================
  Files           665      665              
  Lines         44726    44729       +3     
  Branches       6206     6205       -1     
============================================
+ Hits          43951    43985      +34     
+ Misses          236      216      -20     
+ Partials        539      528      -11
Impacted Files Coverage Δ Complexity Δ
...rnal/operators/observable/ObservableTakeUntil.java 100% <100%> (ø) 2 <1> (ø) ⬇️
...tivex/internal/schedulers/TrampolineScheduler.java 96.1% <0%> (-2.6%) 6% <0%> (ø)
...perators/single/SingleFlatMapIterableFlowable.java 95.83% <0%> (-2.5%) 2% <0%> (ø)
...ernal/operators/flowable/FlowableFromIterable.java 95.18% <0%> (-2.14%) 5% <0%> (ø)
...nternal/operators/observable/ObservableWindow.java 98% <0%> (-2%) 3% <0%> (ø)
...ex/internal/operators/flowable/FlowableCreate.java 96.45% <0%> (-1.3%) 6% <0%> (ø)
...io/reactivex/subscribers/SerializedSubscriber.java 98.86% <0%> (-1.14%) 26% <0%> (-1%)
...perators/observable/ObservableMergeWithSingle.java 99.06% <0%> (-0.94%) 2% <0%> (ø)
...ternal/operators/observable/ObservableFlatMap.java 86.58% <0%> (-0.64%) 3% <0%> (ø)
...internal/operators/flowable/FlowableSwitchMap.java 96.69% <0%> (-0.48%) 3% <0%> (ø)
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d506ddc...66087f4. Read the comment docs.

@akarnokd
Copy link
Member Author

akarnokd commented May 30, 2018

Benchmark results, this PR:

i7 4790, Windows 10 x64, Java 8u172, JMH 1.20

image

The flowable code did not change but my Windows is sometimes quite noisy.

Benchmark                        Mode  Cnt         Score        Error  Units
TakeUntilPerf.flowable          thrpt    5    304957,363    17109,039  ops/s
TakeUntilPerf.flowable:items    thrpt    5  20107899,319  1267248,011  ops/s
TakeUntilPerf.observable        thrpt    5    446404,653    14920,099  ops/s
TakeUntilPerf.observable:items  thrpt    5  20379607,181   699582,594  ops/s

baseline:

Benchmark                        Mode  Cnt         Score       Error  Units
TakeUntilPerf.flowable          thrpt    5    341933,112   13328,573  ops/s
TakeUntilPerf.flowable:items    thrpt    5  21128765,548  677544,832  ops/s
TakeUntilPerf.observable        thrpt    5    366931,108    8202,644  ops/s
TakeUntilPerf.observable:items  thrpt    5  10733841,643  112853,358  ops/s

@akarnokd
Copy link
Member Author

/cc @artem-zinnatullin

@artem-zinnatullin
Copy link
Contributor

89% is very impressive improvement, @akarnokd!

Will try to think about application of similar pattern in my work then, thanks for posting benchmark 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants