-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add post: Lifetime Event Loop #869
Conversation
✅ Deploy Preview for crystal-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I left a few suggestions, plus:
- maybe change the title to "A new Event Loop for UNIX operating systems", so we don't give the impression that we completely overhauled the evloop;
- maybe add a note about
io_uring
(not forgotten, we have plans).
Last but not least: the flag is currently -Devloop
not -Deventloop
. I don't have strong feelings about either, but we need to settle on one.
Mention flags Co-authored-by: Julien Portalier <[email protected]>
This pull request has been mentioned on Crystal Forum. There might be relevant details there: https://forum.crystal-lang.org/t/new-event-loop-unix-call-for-reviews-tests/7207/22 |
The new implementation works well with the multi-threading preview | ||
(`-Dpreview_mt`), having one event loop instance per thread. | ||
|
||
There is on caveat though: Moving file descriptors with pending operations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with the RFC, the breaking change should be shouted out. Also, the distinction between transferring FDs without pending operations should stand. More like "It is possible to transfer a file descriptor between threads as long as there are no pending operations."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This last bit is to not make it sound like trasferring is not possible at all (for the casual reader)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@straight-shoota have you seen this?
Cool, if we removes the |
I consider latter is better? |
No, this doesn't affect that. The trouble with static linking is with the GNU libc itself. Linking |
Yes, it not work.
But, Why it still depends on libevent?
|
The PR mentioned in this draft post has not been merged yet, so the change is not yet available in master/nightly. |
Co-authored-by: Julien Portalier <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also move the section about MT before timers. The program changing behavior is more important than a not that much noticeable performance regression.
Co-authored-by: Beta Ziliani <[email protected]>
This is a draft for announcing the event loop updates once crystal-lang/crystal#14996 is merged.