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

Support Android #269

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Conversation

felixvf
Copy link

@felixvf felixvf commented May 3, 2017

These changes bring basic support for Android from Android API Level 10 onward. This also means that these changes bring basic support for Java 1.6.

Felix von Ferey added 25 commits April 30, 2017 17:40
…JDK version.

This allows to use ThreadLocalRandom under Java 1.6.
…oject which has source code.

In addition, the new dependency also provides ThreadLocalRandom, which is not available in Java 1.6.
@circlespainter
Copy link
Member

circlespainter commented May 6, 2017

Wow that's cool! What does "basic support" mean, have you been able to run Android applications using this modified Quasar, which parts of Quasar and with which degree of success? Also, how about regular Java 1.6 applications? I also wonder how it'd be possible to CI those platforms and also about performance impacts, for example related to the UNSAFE cleanup.

What's your experience so far about the above?

@felixvf
Copy link
Author

felixvf commented May 6, 2017

Wow that's cool! What does "basic support" mean, have you been able to run Android applications using this modified Quasar, which parts of Quasar and with which degree of success?

"basic" means to run quasar fibers.

Also, how about regular Java 1.6 applications?

I have no specific Java 1.6 application to test with (apart from an Android application, which, unfortunately, is not yet open source).

I also wonder how it'd be possible to CI those platforms

It is tricky (especially due to android emulator bugs and Android SDK quirks), but possible. I’ll try to get Android CI working without requiring Android support for regular quasar users. For that to become easier, it would be nice if you could review or merge #267 as well as #268, as these make the current tests less flaky, which is required to make potential CI failures actually show Android specific problems.

and also about performance impacts, for example related to the UNSAFE cleanup.

I have no data, but mostly because I do not know about any benchmarks. Is there any benchmarking code or are there any performance tests available? I’d expect a minor to zero performance impact on optimizing VMs (e.g. HotSpot), as the UNSAFE methods are called under the hood, as evidenced here: http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/43cb25339b55/src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java#l382

What's your experience so far about the above?

With the changes, quasar fibers work without flaws so far.

@circlespainter
Copy link
Member

Have you been able to adapt/run existing tests on Java 1.6 (or even a full build)? It would be nice to have a JDK6 build run in CI.

As for performance tests you could use the JMH ones included in Quasar like this and/or skynet for example.

@pron
Copy link
Contributor

pron commented May 11, 2017

Oh, wow! It will obviously take some time to process, but just something off the bat: why are you using something called jeresy.repackaged (what is it?) rather than rely on shading, as Quasar does now?

@felixvf
Copy link
Author

felixvf commented May 11, 2017

why are you using something called jeresy.repackaged (what is it?) rather than rely on shading, as Quasar does now?

The provided file baselib/jsr166e.jar did not contain class ThreadLocalRandom which quasar requires. However, JDK 1.6 or Android API Level 10 also does not provide ThreadLocalRandom. However, other implementations of JSR166e do provide that class. (Apparently, many slightly differing copies of JSR166e seem to be floating around.) In order to support JDK 1.6 and Android API Level 10, I chose to depend on such an implementation. Because the implementation was already available as Maven-style dependency, shading was not necessary any more. This had the additional benefit that the source of the JSR166e implementation used is actually available (allowing for recompiling and source-level debugging if necessary), that code duplication was reduced (here: the number of slightly differing copies of the JSR166e implementation) and that the burden to maintain the source of the JSR166e implementation was on a different project, increasing the probability that the burden on the quasar project to maintain the JSR166e implementation will be lower in the future.

The reason to choose org.glassfish.jersey.bundles.repackaged:jersey-jsr166e over other implementations was just that it seemed to be the most recent implementation (which indicates that it received the most maintenance) and that it actually contained class ThreadLocalRandom. Apart from that, any copy of JSR166e would have probably fitted.

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