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

Reduce heavy atomic operations on RoutingContextImplBase #2603

Open
franz1981 opened this issue May 5, 2024 · 6 comments
Open

Reduce heavy atomic operations on RoutingContextImplBase #2603

franz1981 opened this issue May 5, 2024 · 6 comments

Comments

@franz1981
Copy link

I see that 0cd7eca from @tsegismont has introduced updaters, but I still don't understand why is using such heavy weight atomic operations there.

Given that is a pretty hot path for quarkus, it would be great to use different ones.
Sending a PR soon, for evaluation

@tsegismont
Copy link
Contributor

@franz1981 sorry for the delayed response. These atomic ops have been removed in #2545 and backported to 4.x in #2546

Vert.x 4.5.2 should be free of them.

@franz1981
Copy link
Author

I can still see them in https://github.com/vert-x3/vertx-web/blob/4.x/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImplBase.java#L157-L158

These are using atomic volatile set(s) in the hotpaths and incrementAndGet which can cost hundreds of times more than normal non atomic operations: are really required? or they could be relaxed?

@franz1981
Copy link
Author

@tsegismont PTAL

@tsegismont
Copy link
Contributor

Which PR?

@franz1981
Copy link
Author

There's no PR, but this comment at #2603 (comment)

In #2603 (comment) it seems the atomics are not there anymore but the atomic operations are still there...

@tsegismont
Copy link
Contributor

There's no PR, but this comment at #2603 (comment)

In #2603 (comment) it seems the atomics are not there anymore but the atomic operations are still there...

Thanks for the heads-up, yes I can see that now. I'll look into it

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

No branches or pull requests

2 participants