Skip to content

1.x: remove remaining field updaters #3979

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

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Jun 1, 2016

This PR removes the remaining AtomicXFieldUpdaters from the library because reflection is somewhat problematic on Android.

This also deprecates the internal BackpressureUtils.getAndAddRequest to indicate requested field updater should not be used anymore.

@zsxwing
Copy link
Member

zsxwing commented Jun 1, 2016

BackpressureUtils.getAndAddRequest

This is an internal API. Nobody is supposed to use it. Why not just remove it?

@akarnokd
Copy link
Member Author

akarnokd commented Jun 1, 2016

External, custom operators may still depend on this utility method.

@@ -45,7 +45,9 @@ private BackpressureUtils() {
* @param n
* the number of requests to add to the requested count
* @return requested value just prior to successful addition
* @deprecated Android has issues with reflection-based atomics
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Android/Samsung/ 😀

Let's not lump all of Android in with the likes of the insane OS developers at Samsung who modify Java system packages needlessly without rhyme or reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's one thing those devices can't find fields but how about reflection in general. Desktop Java does well with inlining and intrinsification of AtomicXFieldUpdaters; I'm not sure about Android in general through.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm not sure. I would imagine the new runtime on Android 5.0+ could do something about it. I'll have to investigate it more.

@zsxwing
Copy link
Member

zsxwing commented Jun 1, 2016

External, custom operators may still depend on this utility method.

Okey, let's remove it later. 👍

@zsxwing zsxwing merged commit ab6dbc1 into ReactiveX:1.x Jun 1, 2016
@stevegury
Copy link
Member

👍

@akarnokd akarnokd deleted the FieldUpdaterRemove branch June 2, 2016 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants