-
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
ScheduledExecutorService: call purge periodically on JDK 6 to avoid #2465
Conversation
cancelled task-retention.
I did run the unit test by forcing a JDK 6 runtime and seems to work. Merging to allow progress on schedulers. |
ScheduledExecutorService: call purge periodically on JDK 6 to avoid
@@ -30,24 +34,111 @@ | |||
private final ScheduledExecutorService executor; | |||
private final RxJavaSchedulersHook schedulersHook; | |||
volatile boolean isUnsubscribed; | |||
|
|||
/** The purge frequency in milliseconds. */ | |||
private static final String FREQUENCY_KEY = "io.reactivex.rxjava.scheduler.jdk6.purge-frequency-millis"; |
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.
These don't match the naming convention using in RxRingBuffer
with just the rx
prefix: https://github.com/ReactiveX/RxJava/blob/1.x/src/main/java/rx/internal/util/RxRingBuffer.java#L267
We should probably stick with that convention since it is already set, so:
rx.scheduler.jdk6.purge-frequency-millis
rx.scheduler.jdk6.purge-force
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.
Okay.
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.
I'd like to fix it in master but then it conflicts with #2579, which by itself also adds parameters with unconventional naming.
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.
Why can't we fix this then go work on #2579 which still needs to be reviewed and can be rebased?
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.
Hystrix could use the enhancements in #2579 and since it affects the same file, it is much easier to change the values there in a plain commit than rebasing.
cancelled task-retention.
First debated in #1922, see also #1919.
We may want to discuss the naming of system parameters. I chose these so RxJava 2.0 specific properties may be trivially separated:
io.reactivex.rxjava.scheduler.jdk6.purge-frequency-millis
Specifies the purge frequency in milliseconds. Default is 1000.
io.reactivex.rxjava.scheduler.jdk6.purge-force
Forces the use of the purge (if set to true) even if the setRemoveOnCancelPolicy is supported. The benefit is that removing cancelled tasks now runs on a different thread so the main pool thread doesn't waste time on them. The drawback is the retention window can be still to large.
Do we have a wiki page where such parameters are listed?