1.x Provide public constant instead of UnsafeAccess.isUnsafeAvailable()

[artem-zinnatullin]

Motivation: save some nanoseconds on JVM and a little bit more on Android, new construction will also be easier for JIT.

[zsxwing]

I guess this single line should be inlined by JIT. Are you worrying about some old Android versions without JIT?

[JakeWharton]

RxJava doesn't support Android devices which lacked a JIT because they only
have Java 5 APIs.

On Thu, Mar 31, 2016 at 3:14 PM Shixiong Zhu [email protected]
wrote:

I guess this single line should be inlined by JIT. Are you worrying about
some old Android versions without JIT?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#3815 (comment)

[artem-zinnatullin]

Yes, if (UnsafeAccess.isUnsafeAvailable()) should be interpreted and inlined in places, but it won't be immediate and JIT will have to figure that out. Also, I think it'll have to do it again for every place where we use this method, but not sure.

Regarding Android afaik: from Android 2.3 to 4.4 it has Dalvik VM that has JIT, from Android 5 to 6 it has ART without JIT, upcoming Android N release will have ART with JIT.

Idea is: if we can do something a little bit more efficiently and it doesn't make code much worse, why not.

[akarnokd]

I don't know.

[artem-zinnatullin]

Totally up to you, it's nanooptimization in internal package, very easy to go back, basically CTRL + R one to another.

// -1 method for Android BTW haha

[ZacSweers]

I think the reason for this should be that it's just a bit cleaner, inlined, and one less method. Perf isn't really a good or measurable heuristic for this. Maybe there's an argument for keeping the method for testing purposes, but I doubt people are mocking this.

[akarnokd]

We have trouble with Samsung devices again. Could you include a check for a system property named rx.unsafe-disable (content value irrelevant) and thus force IS_UNSAFE_AVAILABLE to be false?

[artem-zinnatullin]

@akarnokd to not mix refactoring with new logic I've opened separate PR for system property #3829.

Feel free to merge in any order.

@hzsweers at the moment I don't see any profit in using method, and I'm not sure that somebody is mocking static method (PowerMock?), if I'll need it I'll rewrite value of UNSAFE field via reflection.

[ZacSweers]

Was just chiming in, I doubt anyone us mocking it and it is cleaner so I say go for it.

[stevegury]

In term of performance, I don't think the impact will be measurable (even in interpreted mode), the method will be under the default threshold for Trivial Method (=6 for Hotspot), and will be inlined as soon as the caller is compiled, reference code in OpenJDK.

The only remaining question is, what is cleaner:
UnsafeAccess.isUnsafeAvailable() or UnsafeAccess.IS_UNSAFE_AVAILABLE ?
I prefer the method call.

[zsxwing]

As we may add other logic into isUnsafeAvailable in future (E.g., #3829), I prefer the method call too.

[akarnokd]

I'd keep this as a method as well.

[artem-zinnatullin]

ok