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

Thread.holdsLock calls result in excessive overhead w/o ability to turn it off #5586

Closed
astral303 opened this issue Nov 6, 2019 · 3 comments
Closed
Labels
bug
Milestone

Comments

@astral303
Copy link

@astral303 astral303 commented Nov 6, 2019

When profiling OkHttp3 4.2.1 under a high load (a microservice/RPC scenario), the profiler constantly shows Thread.holdsLock(this) as a non-trivial percentage of wall time spent servicing sending a request.

Here's an example hot spot in Dispatcher.kt:

  private fun promoteAndExecute(): Boolean {
    assert(!Thread.holdsLock(this))

The generated Java source makes the call to Thread.holdsLock(this) even if assertions are off:

  private final boolean promoteAndExecute() {
      boolean var1 = !Thread.holdsLock(this);
      boolean isRunning = false;
      boolean var3 = false;
      boolean var4;
      if (_Assertions.ENABLED && !var1) {

However, Thread.holdsLock is a native call:

public static native boolean holdsLock(Object obj);

The JIT optimizer is unable to optimize across native call boundaries. This cause a hard unable-to-optimize situation, which, for example, inhibits inlining across virtual calls. In my profiling for my target use case, this placed a ceiling on performance that I was not able to work around (save for building a patched OkHttp version).

@astral303 astral303 added the bug label Nov 6, 2019
@JakeWharton

This comment has been minimized.

Copy link
Collaborator

@JakeWharton JakeWharton commented Nov 6, 2019

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

@JakeWharton JakeWharton commented Nov 6, 2019

We can mitigate by duplicating Kotlin/kotlinx.coroutines@583d39d

@yschimke

This comment has been minimized.

Copy link
Collaborator

@yschimke yschimke commented Nov 9, 2019

@JakeWharton this looks worrying i.e. we can't reproduce exactly

// Need InlineOnly for efficient bytecode on Android
@file:Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
@yschimke yschimke added this to the 4.3 milestone Nov 17, 2019
@yschimke yschimke closed this Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.