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

await(Future future) should immediately return or raise if the future is complete - significant perf improvement #5402

Open
doctorpangloss opened this issue Nov 21, 2024 · 0 comments
Labels

Comments

@doctorpangloss
Copy link

doctorpangloss commented Nov 21, 2024

Version

4.5.11

Context

While upgrading my application to 4.5.11 from 4.4.x, I saw a significant performance regression (almost 3x slower) when removing my usage of the vertx virtual threads incubator and replacing with ThreadingModel.VIRTUAL_THREADS. The root cause was that in my fork of the threads incubator, awaiting a Future will check if the future is complete and return immediately or throw. Upstream, the virtual thread always parks. This is inefficient.

Do you have a reproducer?

This implementation of await exhibits the issue (this is just upstream await):

public static <T> T await(Future<T> fut) {
  return io.vertx.core.Future.await(fut);
}

This does not:

public static <T> T await(Future<T> fut) {
  if (fut == null) {
    throw new IllegalArgumentException("Future cannot be null");
  }
  if (fut.isComplete()) {
    if (fut.succeeded()) {
      return fut.result();
    } else if (fut.cause() != null) {
      throwAsUnchecked(fut.cause());
    } else {
      throw new RuntimeException();
    }
  }
  var ctx = Vertx.currentContext();
  if (ctx == null) {
    try {
      return fut.toCompletionStage().toCompletableFuture().get();
    } catch (InterruptedException | ExecutionException e) {
      throw new RuntimeException(e);
    }
  }
  return io.vertx.core.Future.await(fut);
}

Steps to reproduce

The tests in vert-x3/vertx-virtual-threads-incubator#8 previously investigated and reported the issue.

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

No branches or pull requests

1 participant