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

Use non-blocking IO http-client library to send faster and reducce CPU usage #101

Closed
huantt opened this issue Sep 24, 2019 · 8 comments
Closed
Assignees

Comments

@huantt
Copy link

huantt commented Sep 24, 2019

We have a project with a lot of notification must be delivered per hour.
So we're using VertX to send http request asynchronously and pass callback into sendAsync method. Now, we can X2 speed and reducce 1/2 CPU usage by using this library.
If you want to create another driver with asynchronous http request, let contact me to discuss more details
My email: tathuan96@gmail.com

@martijndwars
Copy link
Member

I'd be happy to integrate such improvements in this project (no need for a separate driver). How exactly is your setup (VertX + callback) different from the asynchronous HTTP client that is currently being used by the library? Can you share some code?

@huantt
Copy link
Author

huantt commented Sep 26, 2019

I'd be happy to integrate such improvements in this project (no need for a separate driver). How exactly is your setup (VertX + callback) different from the asynchronous HTTP client that is currently being used by the library? Can you share some code?

Nonblocking IO is different from Nonblocking code.
Look like the driver that use're using this nonblocking code - that use thread pool to do asynchronously.
Can we discuss more about that on Skype or Telegram?

@huantt
Copy link
Author

huantt commented Sep 27, 2019

You can see benchmark here:
https://github.com/huantt/vertx-web-client-benchmark

@yaitskov
Copy link

yaitskov commented Nov 1, 2019

yep. +1

as for now just use PushService.preparePost. it is public and create apache http async client on
your own. It is currently recreated every message while it is thread safe and i guess there are just a few push services in the worlds so safe resources on reusing existing tcp sockets.

@huantt
Copy link
Author

huantt commented Nov 1, 2019

@martijndwars you can read more about non-blocking-io (as how nodejs work in one thread)
It's different from non-blocking-code (non-blocking by using thread pool).
For example: https://vertx.io/docs/vertx-web-client/java/

@sp00m
Copy link
Contributor

sp00m commented Mar 20, 2020

My two cents, I believe the current implementation is non-blocking strictly speaking, if you use PushService#sendAsync, as it uses Apache HttpAsyncClient behind the scene (see their HttpCore tutorial).

The main issue though in my opinion, is that callers can't wait and react on it without blocking, as it returns a good ol' Java5 Future (unless looping over #isDone() in a single-threaded pool so that the final #get() doesn't block, but definitely more a hacky workaround than a proper solution, as it still blocks at the end, meaning contention can still occur).

Ideally, migrating the implementation to a more recent HTTP client that would return a CompletableFuture instead would solve all issues, as reactive libraries like Project Reactor or RxJava (upon which those reactive frameworks rely on) can build their types on top of it.

AHC is a good candidate, here is a migration example: pusher/pusher-http-java#38.

What do you think? Are you happy with me giving this a try if I find some free time?

@martijndwars
Copy link
Member

Thanks for the explanation @sp00m. A PR that makes these changes is much appreciated.

@sp00m sp00m mentioned this issue Jul 3, 2020
@sp00m
Copy link
Contributor

sp00m commented Nov 10, 2020

Now #139 has been merged and released under https://github.com/web-push-libs/webpush-java/releases/tag/5.1.1, I believe this ticket can be closed.

Example with Project Reactor:

PushAsyncService pushService = new PushAsyncService(...);
Notification notification = new Notification(...);
Mono<Response> mono = Mono.fromFuture(() -> pushService.send(notification));

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

4 participants