Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

virtio: consider avoiding dynamic dispatch with enum_dispatch #989

Open
mkroening opened this issue Nov 30, 2023 · 2 comments · May be fixed by #1406
Open

virtio: consider avoiding dynamic dispatch with enum_dispatch #989

mkroening opened this issue Nov 30, 2023 · 2 comments · May be fixed by #1406

Comments

@mkroening
Copy link
Member

mkroening commented Nov 30, 2023

Before 02860f2, there was an enum for abstracting over split and packed virtqueues:

pub enum Virtq {
    Packed(PackedVq),
    Split(SplitVq),
}

Currently, we are using a trait via Rc<dyn Virtq> instead of Rc<Virtq>.

We need to consider automating the previous enum-based static dispatch via enum_dispatch for performance.
This would be a nice fit because restricting the set of implementors is no problem for us.

This might not have a too much of an impact, depending on how hot these dynamic dispatches are right now.

@cagatay-y
Copy link
Contributor

I think we can close this issue since using a trait and trait objects worked out for SplitVq and PackedVqs.

@mkroening mkroening changed the title virtio: adapt enum_dispatch virtio: consider avoiding dynamic dispatch with enum_dispatch Jun 15, 2024
@mkroening
Copy link
Member Author

I think we can close this issue since using a trait and trait objects worked out for SplitVq and PackedVqs.

This issue is about potential performance improvements, not about whether we need it to work at all. I have reworked the description.

This issue might be interesting for @CarlWachter, who will be working on benchmarking.

CC: @jounathaen

cagatay-y added a commit to cagatay-y/kernel that referenced this issue Oct 3, 2024
@cagatay-y cagatay-y linked a pull request Oct 3, 2024 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants