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

feat(driver): move future state into RawOp #251

Merged
merged 17 commits into from
May 21, 2024

Conversation

Berrysoft
Copy link
Member

In this way, no need the OpRuntime, and the HashMap could be removed.

@Berrysoft Berrysoft added package: driver Related to compio-driver package: runtime Related to compio-runtime refactor Refactoring existing code labels May 13, 2024
@Berrysoft Berrysoft self-assigned this May 13, 2024
@Berrysoft
Copy link
Member Author

f6e7c4a adds a large patch to reduce the usage of slab in compio-driver. The allocated pointer itself is a good user-defined data for the operation. On Windows, it is also the pointer of OVERLAPPED struct.

However, the patch makes the code less readable. There are many conversions between pointer and usize, and the RawOp even constructs its own vtable to keep the pointer thin. The performance doesn't increase significantly.

@George-Miao @oxalica What do you think of the patch?

@George-Miao
Copy link
Member

If I'm understanding correctly:

  • RawOp has to be repc(C) and op has to be the last field so that Key<()> will have the same prefix as Key<T>
  • Each upcast::<T> is used as an unique fn ptr to construct the correct dyn vtable from user data for T: OpCode

Is that correct?

@Berrysoft
Copy link
Member Author

Bingo!

@Berrysoft Berrysoft merged commit dbccc93 into compio-rs:master May 21, 2024
29 checks passed
@Berrysoft Berrysoft deleted the refactor/reduce-hashmap branch May 21, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: driver Related to compio-driver package: runtime Related to compio-runtime refactor Refactoring existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants