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

Removes the purge thread in favour of standard ScheduledThreadPoolExecutor APIs #7293

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

chggr
Copy link
Contributor

@chggr chggr commented Jul 15, 2021

The RXSchedulerPurge thread is currently enabled by default and runs every second
to call purge() on each executor in the pool. This is causing significant issues
for battery powered devices (e.g. mobile phones), because it needs to periodically
wake up the CPU to perform purging. The RXSchedulerPurge thread could be completely
removed in favour of using the standard setRemoveOnCancelPolicy() API on the
ScheduledThreadPoolExecutor which became available in Java 7 and offers removal
of cancelled tasks at the moment they are cancelled in O(1).

…utor APIs

The RXSchedulerPurge thread is currently enabled by default and runs every second
to call purge() on each executor in the pool. This is causing significant issues
for low powered devices (e.g. mobile phones), because it needs to periodically
wake up the CPU to perform purging. The RXSchedulerPurge thread could be completely
removed in favor of using the standard setRemoveOnCancelPolicy() API on the
ScheduledThreadPoolExecutor which became available in Java 7 and offers removal
of cancelled tasks at the moment they are cancelled in O(1).
@akarnokd
Copy link
Member

The question is, how does this affect compatibility and desugaring?

/cc @JakeWharton

@JakeWharton
Copy link
Contributor

I do not recall the minimum Android version we decided on for 3.x. It doesn't seem explicitly documented anywhere.

The API in question (setRemoveOnCancelPolicy) is available from Android API 21 and newer (which corresponds to Android 5.0 as a user-facing version). API 21 is a relatively safe minimum, but I suspect prior to this you were able to use 3.x on even lower API levels.

It's not unheard of for libraries to bump their minimum-supported Android API level in minor releases. So I'm supportive for the next minor version provided that the minimum is also documented somewhere.

@akarnokd
Copy link
Member

Okay then, from 3.0.14, we require API 21. I'll add this to the readme.

@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #7293 (331b50c) into 3.x (c5883dc) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #7293      +/-   ##
============================================
+ Coverage     99.53%   99.54%   +0.01%     
+ Complexity     6756     6744      -12     
============================================
  Files           747      747              
  Lines         47391    47347      -44     
  Branches       6382     6376       -6     
============================================
- Hits          47170    47133      -37     
+ Misses          103       96       -7     
  Partials        118      118              
Impacted Files Coverage Δ
...va/io/reactivex/rxjava3/schedulers/Schedulers.java 100.00% <ø> (ø)
...ava3/internal/schedulers/SchedulerPoolFactory.java 100.00% <100.00%> (ø)
...l/operators/observable/ObservableFlatMapMaybe.java 91.54% <0.00%> (-6.34%) ⬇️
.../operators/observable/ObservableFlatMapSingle.java 94.44% <0.00%> (-2.39%) ⬇️
...operators/flowable/FlowableFlatMapCompletable.java 98.71% <0.00%> (-1.29%) ⬇️
...a/io/reactivex/rxjava3/subjects/ReplaySubject.java 99.16% <0.00%> (-0.42%) ⬇️
...3/internal/operators/flowable/FlowableGroupBy.java 84.73% <0.00%> (-0.30%) ⬇️
...ternal/operators/observable/ObservableFlatMap.java 98.22% <0.00%> (ø)
...ernal/operators/flowable/FlowableFlatMapMaybe.java 98.44% <0.00%> (+0.51%) ⬆️
...rnal/operators/flowable/FlowableFlatMapSingle.java 94.18% <0.00%> (+0.58%) ⬆️
... and 6 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 c5883dc...331b50c. Read the comment docs.

@JakeWharton
Copy link
Contributor

Sorry by minor I meant for a 3.1.0 rather than patch release in the 3.0.x series. At least then someone might look a bit more closely at the changes than a patch release where they expect only fixes and not new functionality / changes.

Ultimately you can decide though since it's not explicitly documented anywhere and this is really about making it explicit.

@akarnokd
Copy link
Member

Indeed 3.1.0 would be more appropriate.

@akarnokd akarnokd added this to the 3.1 milestone Jul 15, 2021
@akarnokd akarnokd merged commit 3330943 into ReactiveX:3.x Jul 16, 2021
@chggr chggr deleted the purge branch July 16, 2021 08:14
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.

3 participants