Repository navigation
JCTools as Shaded Dependency #1735
Description
Activity
- added this to the milestone
on Oct 8, 2014 Here is an example of doing this from Couchbase: https://github.com/couchbase/couchbase-jvm-core/blob/master/build.gradle#L166
/cc @nitsanw
@benjchristensen as it turned out, no complaints yet on shading it.
The only thing I found so far is that since we are not shipping the sources for the shaded jars, it's hard to debug and load sources when you try to debug. So I need to figure out how to re-package the shaded sources and ship them with the sources jar so debugging is easy as well.If you know how, I'd be happy to apply it.
JCTools has different philosophy about when a queue is full. For example, trying to put 128 elements into an empty queue in response to a request can yield
MissingBackpressureExceptionbecauseoffer()returns false.@akarnokd part of the joy of development, SpscGrowableArrayQueue was prematurely released, and your help in debugging is appreciated. As discussed on that topic, the Queue interface does not support a capacity() method and the provided capacity in constructors is a open to some interpretation. General attitude in JCTools is that the capacity will be the larger or equal to the capacity prescribed, usually rounded to the nearest power of 2.
Okay, given a power of 2 initial capacity of the queues, RxJava expects them to hold that amount of values exactly if there are no consumers.
I wonder if having a capacity() method which lets you know what the actual capacity it would be a help?
I'm planning to release new Queue interfaces with the next version of JCTools.You misunderstand me. Any JCTools queue that we use has to pass the following test.
I understand, but I'm not sure I agree :-)
Just because our app needs this use case does not mean the underlying queue needs to support it outside the power of 2.
@akarnokd We can address this just by making it so our sizes are always a power of 2. This is why I've always stuck to a power of 2 for our sizing so that we don't have to layer an extra check on top and can just rely on
offerto reject.20 remaining items
A PR is welcome setting this up, however, there are 3 problems that need addressing:
- We don't need all of JCTools' queue implementations; can the shading include/exclude classes?
- Currently, our SpscUnboundedArrayQueue offers a two argument offer(T, T) - no varargs, no lambdas.
- Some Android devices appear to be supporting Unsafe but then they crash at some point. We address this via a disable flag. Since we'd drop our UnsafeAccess, the flag handling needs to be updated as well.
This was successfully done at Netty:
- Shade plugin has an option to only include used classes.
- There are some atomic versions of these queues.
I would send a PR, but my Gradle knowledge is at most basic to get to the point of using a plugin, it was difficult enough in Maven because we had to also make the dependency optional for OSGI and available for tests on the parent pom.
For single producer/consumer, I would use the atomic version because one thread won't race against anything else hence no making a performance difference against
Unsafe; the real difference when usingUnsafewill come when using either multiple producer or consumer or both.@akarnokd I know the Gradle shade plugin is slightly flakier than the maven one, so that might prove a blocker here. You should be able to make this work using explicit includes/excludes.
The double arg SPSC is a usecase I plan to support via code generated channels, for now you might have to extend the class somehow or keep your variant.@guidomedina The difference between Unsafe/Atomic performance is down to indirection costs. I think your intuition on this is incorrect. There's no difference in contention in principal, in fact using atomics may reduce contention because having slower consumer/producer code means the contended memory is hit less frequently.
@nitsanw there are more ops to be made that I agree, but in practice you get a 100% hit rate on get, add or both hence making the difference in cost to tend to zero, I agree that my previous statement technically speaking was wrong but the practical difference is what?
That in the scenario of only one thread at head and tail concurrently.
@guidomedina please reflect your intended usecase in a JMH benchmark, let the numbers speak ;-)
Reacted by Xiaolin JuRatio using Java8u92 i5-4670k after
Unsafehas warmed up is 0.4 indicating for a single thread it is twice faster, but is that the same for Android?Post your benchmark as gist please.
Why do you think Unsafe is 2 times faster? Use -prof perfnorm and reason on counters?On 18 Jun 2016, at 17:31, Guido Medina notifications@github.com wrote:
Ratio using Java8u92 i5-4670k after Unsafe has warmed up is 0.4 indicating for a single thread it is twice faster, but is that the same for Android?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.Note that Netty now properly depends on JCTools. Also note recent improvements to SPSC linked array queues (not implemented in the atomic cases).
// Sadly I see no good way to express the control flow below using inheritance.
I saw you unified a lot of implementations. I'd kept them separate so no excess but unused features sneak
into the hot path. In addition, there are now extra fields and local variables which taxes the JIT and may not leave room for local variables outside the offer/poll.Plus it still doesn't have two object offer which saves us a tuple allocation / spinning on a poll.
Btw, I'd be interested in a blog post about how the JIT's register allocator behaves when there are more locals than registers. I had odd and lower benchmark results in some drain loops: when I cached a final field, that was slower than reading the field in a loop, surrounded by unrelated volatile reads.
@akarnokd I'm happy to discuss, but not all of your comment seems focused on this thread
"Plus it still doesn't have two object offer which saves us a tuple allocation / spinning on a poll." -> trueUnrelated:
"I saw you unified a lot of implementations. I'd kept them separate so no excess but unused features sneak into the hot path. " -> This is all in the offerColdPath. This is as bad as a bimorphic call, so no worse than inheritance, just uglier. There's 2 unused fields in the Unbounded case. Because there's no clean way to extend a class with layout control (I want extra fields in the producer fields clamp), this may not be easily avoidable. Can maybe reorder the inheritance order and add them in extension.
The implementation for Growable is much much simpler, and has a lot in common with the chunked, I think that worked out well. Overall code size has reduced. I've re-benchmarked and the results look good...
As for number of var/fields impact on register allocation, while in theory larger methods can confuse the register allocator in principal the number of registers on a typical CPU is quite high and running out of registers is not that common. The case you describe should be examined in isolation. I'd be happy to have a look.Closing as this will likely not happen for 1.x. Subsequent versions have to use customized versions anyway.
Now that JCTools is available on Maven Central (http://search.maven.org/#search%7Cga%7C1%7Cjctools) we should consider depending on it directly instead of copy/pasting as we have done so far: https://github.com/ReactiveX/RxJava/tree/1.x/src/main/java/rx/internal/util/unsafe (note the README that provides background and details).
There are a few considerations:
RxJava is a "zero dependency" project so the dependency will need to be shaded (package names changed to be inside RxJava) so that it becomes part of the RxJava jar.
Byte size of pulling in code from JCTools that we don't need. This affects Android particularly.
Use of JCTools must all be done behind a conditional check
UnsafeAccess.isUnsafeAvailable()otherwise RxJava will not work on platforms withoutsun.misc.Unsafe.