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

Document Fiber::SchedulerInterface#io_read and #io_write #5280

Merged
merged 3 commits into from Dec 20, 2021

Conversation

zverok
Copy link
Contributor

@zverok zverok commented Dec 15, 2021

Screenshot (ignore #io_wait):
image

@zverok
Copy link
Contributor Author

zverok commented Dec 15, 2021

@ioquatix Please review. Should other details be mentioned? Is the description correct overall?

@ioquatix
Copy link
Member

This looks good.

io_read

The length argument is the "minimum length to be read".

If the IO buffer is big, e.g. 8KiB, but the length is 1KiB, up to 8KiB might be read, but at least 1KiB will be.

io_write

The length argument is the "[maximum] length to be written".

If the IO buffer is big, e.g. 8KiB, but the length is 1KiB, at most 1KiB will be written. Generally, the only case where less data can be written is if there is an error writing the data.

@ioquatix
Copy link
Member

Please mark/tag io_write and io_read as experimental.

@ioquatix
Copy link
Member

I had to make some internal changes to the hooks, I will have some updates to this documentation in a day or two.

#5287

@zverok
Copy link
Contributor Author

zverok commented Dec 17, 2021

@ioquatix Is that what you had in mind? I am not sure—maybe I am lost a bit—whether the new interface is expected to return "length or -errno" (how I described it) or [-errno, length] (hmm, that's what your PR's title says?..)

image

@ioquatix
Copy link
Member

Yes, this interface can return an integer in the range of [-errno, length].

I provided C interface to hide exact implementation details of the VALUE returned: https://github.com/ruby/ruby/pull/5287/files#diff-1552659974ee1047f418def875393a583c6a8c543e446755a52acdcae5467fc1R28-R47

FYI Linux internally follows this model and libc writes the return value to errno: https://yarchive.net/comp/linux/errno.html from Linus himself.

cont.c Outdated Show resolved Hide resolved
cont.c Outdated Show resolved Hide resolved
@ioquatix
Copy link
Member

@zverok thanks so much for helping with this.

Copy link

@bryanp bryanp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This is great.

@zverok
Copy link
Contributor Author

zverok commented Dec 17, 2021

@ioquatix I've applied your suggestions, thanks! 🤗
I also added two more methods docs (wanted to do in a separate PR, but it actually makes more sense to handle it all once and for good):

#address_resolve
image

#timeout_after
image

@ioquatix
Copy link
Member

I wonder if it would make sense to move this pseudo-implementation to scheduler.c just to avoid making cont.c more burdened.

cont.c Outdated Show resolved Hide resolved
@ioquatix
Copy link
Member

@zverok I've asked the author of address_resolve hook @bruno- to provide/update the documentation.

cont.c Show resolved Hide resolved
cont.c Show resolved Hide resolved
cont.c Show resolved Hide resolved
@ioquatix ioquatix merged commit 711342d into ruby:master Dec 20, 2021
@nobu nobu added the Documentation Improvements to documentation. label Dec 21, 2021
@zverok zverok deleted the docs-scheduler-interface branch December 21, 2021 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements to documentation.
5 participants