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

Discuss relaxing API to support NIO in the future #623

Open
gatesn opened this issue Apr 8, 2020 · 3 comments
Open

Discuss relaxing API to support NIO in the future #623

gatesn opened this issue Apr 8, 2020 · 3 comments

Comments

@gatesn
Copy link

gatesn commented Apr 8, 2020

What happened?

Currently the Channel interface returns a ListenableFuture<Response> and a Response object has an InputStream body. This makes it hard (impossible?) to efficiently implement NIO in the future.

What did you want to happen?

Should we break the Response interface now to parameterise the body type? This more closely reflects the Java11 HttpResponse object and would (I think? not yet too familiar with this) allow us to make better use of non-blocking HTTP clients.

e.g. for the Java HTTP client, we could continue to convert back to InputStream (with BodySubscribers#ofInputStream) then perhaps move to using the Jackson non-blocking parser interface in the future.

@iamdanfox
Copy link
Contributor

iamdanfox commented Apr 8, 2020

I like the idea of trying to make dialogue NIO-friendly before we get too locked in.

Reading through the BodyHandlers.ofString(), it seems that the java client passes data by calling a void onNext(List<ByteBuffer> items) method and then finally calling onComplete(). I'm not 100% sure why they pass a List<ByteBuffer> rather than just calling the method multiple times, but there probably a perf optimization somewhere.

One thing to keep in mind though is that it would probably be lame to make our ApacheHttpClient calls (which expose blocking APIs) translate from InputStream -> some async flowable thingy and then translate back -> InputStream.

@gatesn
Copy link
Author

gatesn commented Apr 8, 2020

We could use the AsyncApacheHttpClient, but I think you're right, we don't want to take a perf hit for blocking http clients.

What would be the next steps, perhaps pushing up a prototype using a non-blocking http client to see where things fall apart?

@jkozlowski
Copy link
Contributor

@iamdanfox likely optimisation to reduce number of syscalls.

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

No branches or pull requests

3 participants