Skip to content

2.0 Design: Performance Benchmarking #2784

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

Closed
benjchristensen opened this issue Feb 28, 2015 · 6 comments
Closed

2.0 Design: Performance Benchmarking #2784

benjchristensen opened this issue Feb 28, 2015 · 6 comments
Milestone

Comments

@benjchristensen
Copy link
Member

Right from the start of RxJava 2.0 we should get performance benchmarking in place. Preferably we will get a mechanism in place that audits for performance regressions (RxJava v1 does not have this, we do it manually).

We need to leverage this while solving things like #2780 and #2782 which both prevent full performance in RxJava v1 due to extra object allocations and requirements for volatiles.

@benjchristensen benjchristensen added this to the 2.0 milestone Feb 28, 2015
@benjchristensen
Copy link
Member Author

A goal for RxJava 2.0 is that nothing prevents it from being used in high-throughput, low-latency use cases.

For example, the SafeSubscriber and concurrent unsubscribes of RxJava 1.0 limit the total throughput due to volatile checks and extra object allocations.

@benjchristensen
Copy link
Member Author

As an implementation note, I've been happy with JMH and the community around it. I suggest we continue with JMH at the /src/perf folder model so they live with the code just like the unit tests.

@akarnokd
Copy link
Member

So far with my reimplementation, I found these things that affect the performance characteristics:

  • RS requires conformance checking on each Subscriber methods, which affects onNext mostly: one has to check if the upstream subscription is available and check if item is non-null. Since most Subscriber activity need to be wrapped by Safe, Serialized or Cancellable, we can't really avoid the checks multiple times but only hope JIT will recognize the redundancy in item null-checks
  • Most Subscription communication doesn't need any synchronization or volatile field. SerializedSubscriber serializes the setting of the Subscription and uses synchronized blocks. CancellableSubscriber uses volatile to handle the Subscription's atomicity requirement. Unfortunately, it seems CancellableSubscriber will be required quite often and because external cancellation is required, there is no way to avoid atomics related to it.
  • Subscription.request() is either wrapped or forwarded without change so no performance loss there, but sources may still require to have thread-safe handling of request and cancel. In my AbstractSubscription I combined the notion of request amount and cancelled state into a single atomic long. Maybe the request(n) calls are always sequential and only cancel has to be thread-safe; I can't verify this yet. However, RS §3.6 requires that request after cancellation should be no op, but cancel is async hence the combined atomic long.
  • Reimplementing from scratch (instead of copying) and using field atomics almost everywhere (UNSAFE now, VarHandles as soon as it becomes available) may reduce object allocation.
  • We are using an outdated JMH version and it comes from a nebula plugin. I've asked for an update on their page but no response yet. @benjchristensen can you influence this?

@benjchristensen
Copy link
Member Author

Most Subscription communication doesn't need any synchronization or volatile field.

This will be a big deal based on my past perf testing (#1383)

sources may still require to have thread-safe handling of request and cancel.

Wouldn't this be dependent on the source though? Most of the time it is synchronous emission on a given thread so that shouldn't need volatile/synchronized behavior.

Similar to cancellation coming from an operator. A takeUntil(Observable) operator that accepts an external Observable would need to provide thread-safety, but takeUntil(predicate) would not.

outdated JMH version

I have pinged them via email.

conformance checking

Is there anything that is too burdensome and requires us pushing back on the RS spec?

@akarnokd
Copy link
Member

akarnokd commented Mar 5, 2015

Most trivial Subscriptions (which are either forwarded downstream or the operator only calls its methods in-sequence from onXXX methods) don't need to add any extra synchronization.

Again, it is a battle between biased locking and atomics: if everything is synchronous and non-thread-hopping, the usual serialization

synchronized (this) {
    if (emitting) {
        missed = true;
        return;
    }
    emitting = true;
    missed = false;
}

is much cheaper than an AtomicInteger.incrementAndGet(), however, once locks become unbiased, the atomics gives much better throughput/latency for non-first callers.

Making sure our internal behavior conforms to the RS-spec is low-to-medium annoying, but it helps in discovering bugs easier (i.e., seeing an NPE stating the rule violation helps me a lot). I'd say, in our entry (from-producer) and exit point (subscriber) to the Observable, we make sure the behavior conforms the RS-spec to the letter (by sinking errors into an uncaught handler and defensively serializing the Subscription).

@akarnokd
Copy link
Member

2.x is rearchitected and allocates far less than 1.x. Other algorithmic changes resulted in 2x-4x lower overhead in some cases.

The gradle file has support for running JMH benchmarks, few of them ported from 1.x. I'm not too keen on porting all benchmarks and those can be added at anytime later in case some odd performace profile comes back from the users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants