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

NIOTS doesn't let you control the equivalent of maxMessagesPerRead and recvAllocator #92

Open
weissi opened this issue Jun 9, 2020 · 7 comments

Comments

@weissi
Copy link
Member

weissi commented Jun 9, 2020

// TODO: Can we do something sensible with these numbers?
self.outstandingRead = true
conn.receive(minimumIncompleteLength: 1, maximumLength: 8192, completion: self.dataReceivedHandler(content:context:isComplete:error:))

NIOTS always does one underlying receive per read and always up to 8192.

So that kinda corresponds to:

  • maxMessagesPerRead = 1
  • recvAllocator = AdaptiveRecvAllocator(minimum: 1, maximum: 8192, initial: ???)

We should make those values configurable.

@edwellbrook
Copy link

Hi @weissi! Just came across this in one of my own projects and have some motivation to fix it.

This would be my first time contributing to to an swift-nio project, though I have some basic understanding of how things work. Some guidance would be appreciated...

Could you suggest what a good user-facing API would look like for configuring these values? Perhaps another option under NIOTSChannelOptions?

Thanks!

@weissi
Copy link
Member Author

weissi commented Apr 16, 2021

Hi @edwellbrook , awesome, and welcome! And thank you for your PR, I'll leave comments regarding your PR on the actual PR.

Were you separately from your existing PR (which configures the maximum read size) interested in working on maxMessagesPerRead?

@edwellbrook
Copy link

Unfortunately I don't have much need for maxMessagesPerRead so I can't budget the time for it right now. If my existing PR needs updating I might be able to quickly fit this in though!

Not super familiar with maxMessagesPerRead so would need a suggestion on how it's supposed to work and should be implemented.

@weissi
Copy link
Member Author

weissi commented Apr 21, 2021

@edwellbrook totally up to you. I fully understand if you can't budget the time. Ping me if you'd like an explanation of what it does exactly and how we could implement it.

@edwellbrook
Copy link

@weissi It's looking like I may need maxMessagesPerRead after all (if I'm understanding it correctly).

Does it refers to the number of messages an InboundChannelHandler will receive from a read call before needing to call read again? i.e. setting maxMessagesPerRead to 1 would mean an InboundChannelHandler needs to call read for each message?

If that's right... I'd love a suggestion on how to implement it.

Cheers!

@Lukasa
Copy link
Contributor

Lukasa commented Apr 21, 2021

Yup, that's exactly what it means.

In principle the mechanism to implement it should be simply to call .receive multiple times. I'm not 100% confident the code will handle that well, but that's how I'd go about starting out.

@edwellbrook
Copy link

@Lukasa Thanks! Will take a look tomorrow morning.

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