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

Review math inlining #493

Closed
kpreid opened this issue May 25, 2024 · 2 comments
Closed

Review math inlining #493

kpreid opened this issue May 25, 2024 · 2 comments
Labels
kind: performance Ways to increase performance or efficiency without adding functionality

Comments

@kpreid
Copy link
Owner

kpreid commented May 25, 2024

In commit 45a772d I split the math and raycast code into a new library crate, all-is-cubes-base. Unfortunately, I didn’t think at the time about the effect this would have on function inlining. rustc currently has a heuristic for marking small functions inlinable, but it doesn't seem to suffice. After a slapdash addition of #[inline] where it seemed like it would make sense, I get the following notable results in benchmarks:

Bench target Benchmark name Effect of inlining on run time
space space-bulk-mutation/* -0%..-96%
space GridAab/iter_for_each -42%
save load -21%
mesh block/* -10%..-26%
raytrace threaded/* +20%..+29%
mesh space/* +7%..+41%
block evaluate/* +16%..+56%
raycast many steps * +85%

Benchmarks run on my laptop have quite unstable results, but these are huge differences which I am confident are significant. But since they are differences in both directions, we need to break down the #[inline] changes into smaller units to determine what's actually improving performance and what's making it worse.

@kpreid kpreid added the kind: performance Ways to increase performance or efficiency without adding functionality label May 25, 2024
@kpreid
Copy link
Owner Author

kpreid commented May 25, 2024

The above data was collected by running all benchmarks only once. On re-testing, it seems that a lot of the apparent losses (and gains) may have been noise. Sigh.

kpreid added a commit that referenced this issue May 26, 2024
These changes have been tested to be neutral-to-positive on benchmarks.
I'm incrementally testing inlining of `all-is-cubes-base` functions
because doing so broadly (as an attempt to recover performance lost
in the crate split) has proven to be problematic — see details:
<#493>
@kpreid
Copy link
Owner Author

kpreid commented May 29, 2024

5c90bca puts in the rest of the inlining. In the end, I couldn't find any consistent regression.

@kpreid kpreid closed this as completed May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: performance Ways to increase performance or efficiency without adding functionality
Projects
None yet
Development

No branches or pull requests

1 participant