Skip to content

Use a macro for defining benchmarks #1464

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

Merged
merged 1 commit into from
Oct 13, 2022
Merged

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Oct 12, 2022

This is done mostly for forward compatibility. In the future, it might make sense to normalize the benchmark functions (e.g. apply #[inline(never)] to them, align them to some boundary to reduce code alignment issues etc.). This can be a bit difficult to do with closures, maybe we will need to generate actual functions.

For that reason, I suggest to change benchmark definition to a macro. That will allow us to easily change the inner representation in the future without needing to change the code of all runtime benchmarks. This PR introduces the macro, but still keeps benchmarks as closures.

@Kobzol Kobzol requested a review from nnethercote October 12, 2022 19:37
@Kobzol Kobzol force-pushed the runtime-define-benchmark branch from c3d02bd to 04344a2 Compare October 12, 2022 21:08
Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

Seems fine, though addressing the question above might make things a little nicer.

@Kobzol Kobzol force-pushed the runtime-define-benchmark branch from 04344a2 to 2df61ca Compare October 13, 2022 13:28
@Kobzol Kobzol enabled auto-merge October 13, 2022 19:32
@Kobzol Kobzol merged commit a987a52 into master Oct 13, 2022
@Kobzol Kobzol deleted the runtime-define-benchmark branch October 13, 2022 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants