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

WIP: Add evented write support #3907

Closed
wants to merge 4 commits into from

Conversation

frmdstryr
Copy link
Contributor

@frmdstryr frmdstryr commented Dec 14, 2019

This is for #3557. "It works" but currently disables the ability to use a custom value.format.

It just adds separate "sync" functions while leaving the default functions async when using .evented mode.

The value.format needs to have another parameter so it can select which output functions to use, this compiles and runs but it reaches unreachable code so something's not right there (so I've disabled it for now). Edit: The error appears to be unrelated.

I'm not sure what the implications of wrapping every output fn.

Also this is probably not the best way to do it so feel free to close this.

Edit: By "it works" I mean a simple StreamServer using .evented.

@andrewrk
Copy link
Member

I've been tackling this same problem a slightly different way. My goal is to turn std.fmt.format into a function that returns an iterator rather than using callback functions. This makes it not possible to fail and makes it no longer use callbacks. This makes it always blocking regardless of I/O mode.

The limitation is that formatting facilities have to have two modes - the first mode returns a compile time known number of upper bound buffer bytes it needs. The second mode is given a buffer with the requested size and it has to return an iterator that yields slices.

I'm still experimenting with this, and it led to me working on a simplification of internal compiler details of how comptime code interacts with result locations. I'm in the middle of that, which is not working yet but if I get it working, a lot of the annoying result location hacks are deleted in this branch.

@@ -318,8 +327,8 @@ pub fn read(fd: fd_t, buf: []u8) ReadError!usize {
EINTR => continue,
EINVAL => unreachable,
EFAULT => unreachable,
EAGAIN => if (std.event.Loop.instance) |loop| {
loop.waitUntilFdReadable(fd);
EAGAIN => if (is_async and std.event.Loop.instance != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this change? As far as I can tell nothing is different here. When that value is null it is comptime known to be null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it like that so even if there's an event loop it can still be forced to use a blocking version (to fix the thread startup using extern).

Copy link
Member

Choose a reason for hiding this comment

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

If the file descriptor is known to be opened in blocking mode, then the best thing to do is noasync fn calls. This way the same (async) function is used, but bloat of an identical non async function is avoided, and it does not cause the caller to be async.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. So noasync os.write() would work even if os.write is async?

I more or less did this to see what impact it had on the server performance. It didn't seem to change much (which makes sense for high load since it's not io bound).

@frmdstryr frmdstryr closed this Dec 31, 2019
@fengb fengb mentioned this pull request Jan 4, 2020
10 tasks
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

Successfully merging this pull request may close these issues.

None yet

2 participants