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

perf(vm): use dynamic dispatch to enable/disable tracing of instructions #35

Merged
merged 9 commits into from
Oct 26, 2023

Conversation

nils-mathieu
Copy link
Collaborator

@nils-mathieu nils-mathieu commented Oct 26, 2023

This PR introduces a dynamic dispatch mechanism to trace instructions within the VM.

This is related to #25.

Performance Considerations

I'm not yet sure whether a function pointer call is that slower or faster than a regular if/else branch. The CPU might be able to optimze the latter better because of its knowledge of the content. With this PR, we might've just removed any optimization possibility entierly.

This should be benchmark to avoid regressions.

@nils-mathieu
Copy link
Collaborator Author

nils-mathieu commented Oct 26, 2023

This remains in draft until I can figure out whether this is a regression or not :) This kind of optimization is really hard to predict.

@nils-mathieu nils-mathieu marked this pull request as ready for review October 26, 2023 14:38
Copy link
Collaborator

@AbdelStark AbdelStark left a comment

Choose a reason for hiding this comment

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

left some comments

src/cmd/cmd.zig Outdated Show resolved Hide resolved
src/cmd/cmd.zig Outdated Show resolved Hide resolved
src/vm/trace_context.zig Show resolved Hide resolved
Copy link
Collaborator

@AbdelStark AbdelStark left a comment

Choose a reason for hiding this comment

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

small comment

src/cmd/cmd.zig Outdated Show resolved Hide resolved
Copy link
Collaborator

@AbdelStark AbdelStark left a comment

Choose a reason for hiding this comment

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

lgtm

@AbdelStark AbdelStark merged commit 4387bd2 into keep-starknet-strange:main Oct 26, 2023
4 checks passed
Copy link

Pull Request closed and locked due to lack of activity.
If you'd like to build on this closed PR, you can clone it using this method: https://stackoverflow.com/a/14969986
Then open a new PR, referencing this closed PR in your message.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants