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

refactor: simplify attach logic #200

Merged
merged 29 commits into from
Mar 4, 2024

Conversation

Berrysoft
Copy link
Member

@Berrysoft Berrysoft commented Jan 29, 2024

io-uring and polling both allow an IO source being registered with multiple drivers, while IOCP doesn't allow that. Internally, IOCP handle is recorded inside the file handles and socket handles; try_clone only increases the reference count of the file/socket handle, which means IOCP handle still remains the same. Therefore, one IO handle can only be attached to one IOCP handle.

If we want to send the IO source to other threads, we must be very careful to make sure the handles have not been attached. That's what Unattached for. It is too complicated to use.

This PR spawns a global thread to query IOCP. The thread checks the driver id and wake up the complete one. This synchronization is backed by win32 Event. Should point out that win32 Event could not be used directly in GUI thread, because it would block the message loop; but it is also not encouraged to perform IO in the GUI thread:)

Closes #202

@Berrysoft Berrysoft added driver: IOCP About the IOCP driver package: driver Related to compio-driver refactor Refactoring existing code labels Jan 29, 2024
@Berrysoft Berrysoft self-assigned this Jan 29, 2024
@Berrysoft Berrysoft marked this pull request as draft January 29, 2024 03:30
@Berrysoft Berrysoft marked this pull request as ready for review January 29, 2024 04:00
@Berrysoft
Copy link
Member Author

Need a multi threaded benchmark to check if there's any performance regression.

@Berrysoft
Copy link
Member Author

Win32 event may not reliable. I'll try CondVar

@Berrysoft Berrysoft marked this pull request as draft January 31, 2024 13:43
@Berrysoft Berrysoft marked this pull request as ready for review January 31, 2024 16:10
@Berrysoft Berrysoft marked this pull request as draft February 3, 2024 12:06
@Berrysoft Berrysoft marked this pull request as ready for review February 3, 2024 16:23
@Berrysoft
Copy link
Member Author

This PR proposes a new feature iocp-global. While the feature is enabled, only one IOCP is created, and the loop is performed in a separate thread. In this case, every IO resources could be safely sent to any threads.

While iocp-global is disabled, IOCP is created per proactor. The overlapped structure records the correct proactor that receives the operation, and if the received entry doesn't belong to the current IOCP, it will try to repost the entry to the correct port. IO resources could still be sent safely to any threads, but be careful. If the attached runtime is inactive or blocked, even if the resources is running on another runtime, any overlapped operation will be blocked, too.

@Berrysoft Berrysoft marked this pull request as draft February 3, 2024 16:56
@Berrysoft Berrysoft added bug Something isn't working and removed refactor Refactoring existing code labels Feb 3, 2024
@Berrysoft Berrysoft marked this pull request as ready for review February 4, 2024 03:19
compio-driver/src/iocp/mod.rs Show resolved Hide resolved
compio-runtime/src/attacher.rs Show resolved Hide resolved
compio-runtime/src/attacher.rs Outdated Show resolved Hide resolved
compio-runtime/src/attacher.rs Show resolved Hide resolved
compio-runtime/src/attacher.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@Berrysoft Berrysoft added this to the v0.9 release milestone Feb 19, 2024
@Berrysoft Berrysoft force-pushed the refactor/simplify-attacher branch 2 times, most recently from 709b86b to d293b27 Compare February 25, 2024 09:22
Copy link
Member

@George-Miao George-Miao left a comment

Choose a reason for hiding this comment

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

LGTM

@Berrysoft Berrysoft merged commit b63fc5c into compio-rs:master Mar 4, 2024
29 checks passed
@Berrysoft Berrysoft deleted the refactor/simplify-attacher branch March 4, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working driver: IOCP About the IOCP driver package: driver Related to compio-driver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsoundness with IOCP driver
2 participants