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

controlling channel buffer maximum I/O size #19962

Closed
jhh67 opened this issue Jun 8, 2022 · 4 comments
Closed

controlling channel buffer maximum I/O size #19962

jhh67 opened this issue Jun 8, 2022 · 4 comments

Comments

@jhh67
Copy link
Contributor

jhh67 commented Jun 8, 2022

Issue #18913 modifies the channel buffer so that there is a limit to the size of the I/Os between the buffer and the underlying file. This behavior is not controllable by the application, however. Should we:

  1. Modify channel.init with an optional parameter that sets the maximum I/O size? If so, what should its name be?
  2. And/or should there be channel methods that allow the application to set and get the maximum I/O size? If so, what should their names be?
  3. What should the default maximum I/O size be? Note that one possibility is "unlimited" which is backwards compatible with the current behavior.
@mppf
Copy link
Member

mppf commented Jun 9, 2022

Issue #18913 modifies the channel buffer so that there is a limit to the size of the I/Os between the buffer and the underlying file.

Did you mean to mention a PR?

I thought this issue would be about the size of the buffer chunks that the qio system allocates, which is currently 64KiB, and not configurable (except by undocumented means). But, it sounds like it is talking about the amount of memory that can be buffered with mark etc. Am I understanding this correctly?

@jhh67
Copy link
Contributor Author

jhh67 commented Jun 9, 2022

I created draft PR #19967 for this issue, although tbh i'm never sure when to create a PR and when not to. In this case there are still unresolved design decisions so a PR seems premature but I went ahead and created one anyway. I made it a draft because it's not ready for review until these design decisions have been finalized.

The issue here is not the size of the buffer chunks, but the size of the I/Os between the buffer and the underlying file, which in turn affects the size of the copies between the application and the channel buffer, and therefore the total amount of channel buffer required. Previously, doing a read of N bytes into a string or bytes required 3N bytes of memory, while doing a write of N bytes of a string or bytes required 2N bytes of memory. With these changes reads and writes require N+d bytes of memory where d is the size of the I/O (rounded up to a whole number of buffer chunks).

@mppf
Copy link
Member

mppf commented Jun 10, 2022

I don't think that it needs to be configurable at all. I think choosing a value between 4KB and 64KB would probably be fine.

@jhh67
Copy link
Contributor Author

jhh67 commented Jun 28, 2022

Closing this as there isn't currently a desire to make this configurable by the application. It shouldn't be hard to add this functionality if we subsequently decide it's needed. The default is currently 64KB.

@jhh67 jhh67 closed this as completed Jun 28, 2022
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

2 participants