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: Fix the extra retention problem in ReplaySubject #5892

Merged
merged 2 commits into from
Mar 7, 2018

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Mar 6, 2018

In the bounded buffers of ReplaySubject, the head reference may retain one extra item when the trimming happens. Nulling out this reference is not possible at this point because old consumers may be still walking through the underlying linked list of nodes. However, replacing a head with the same next pointer (which is always not null if value is not null) but no value will eventually let the value get GCd. This cleanup doesn't happen on every onNext because it doubles the node allocation and thus the overhead.

This PR modifies the code so that terminal events do perform this head swapping and introduces the ReplaySubject.cleanupBuffer() method to allow the user to perform the head swapping while the ReplaySubject is not yet terminated and the cleanup is needed.

If this type of change is accepted, the ReplayProcessor can also be refitted. For the replay() operators, the terminal cleanup can be implemented but the on demand cleanup can't as there is no API surface for its internal buffer available.

@akarnokd akarnokd added this to the 2.2 milestone Mar 6, 2018
@codecov
Copy link

codecov bot commented Mar 6, 2018

Codecov Report

Merging #5892 into 2.x will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5892      +/-   ##
============================================
+ Coverage     97.92%   97.94%   +0.01%     
+ Complexity     5985     5984       -1     
============================================
  Files           655      655              
  Lines         43836    43863      +27     
  Branches       6072     6076       +4     
============================================
+ Hits          42926    42960      +34     
  Misses          278      278              
+ Partials        632      625       -7
Impacted Files Coverage Δ Complexity Δ
...main/java/io/reactivex/subjects/ReplaySubject.java 97.69% <100%> (-0.31%) 50 <1> (+1)
...ternal/operators/observable/ObservablePublish.java 92.98% <0%> (-3.51%) 11% <0%> (ø)
...ava/io/reactivex/processors/BehaviorProcessor.java 95.96% <0%> (-3.14%) 59% <0%> (-1%)
...in/java/io/reactivex/subjects/BehaviorSubject.java 96.23% <0%> (-2.69%) 54% <0%> (ø)
...activex/internal/observers/QueueDrainObserver.java 97.43% <0%> (-2.57%) 21% <0%> (-1%)
...l/operators/observable/ObservableFlatMapMaybe.java 92.81% <0%> (-1.97%) 2% <0%> (ø)
...a/io/reactivex/internal/util/QueueDrainHelper.java 97.22% <0%> (-1.39%) 56% <0%> (-1%)
...ternal/operators/completable/CompletableMerge.java 97.61% <0%> (-1.2%) 2% <0%> (ø)
...operators/observable/ObservableMergeWithMaybe.java 99.1% <0%> (-0.9%) 2% <0%> (ø)
...perators/mixed/ObservableConcatMapCompletable.java 99.11% <0%> (-0.89%) 2% <0%> (ø)
... and 19 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 cb05a26...2b1f767. Read the comment docs.

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.

2 participants