Inline trivial methods in bevy_hierarchy #11332
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In bevyengine#11330 I found out that `Parent::get` didn't get inlined, **even with LTO on**! Not sure what's up with that, but marking functions that consist of a single call as `inline(always)` has no downside. `inline(always)` may increase compilation time proportional to how many time the function is called **and the size of the function marked with `inline`**. Since we mark as `inline` no-ops functions, there is no cost to it. I also took the opportunity to `inline` other functions. I'm not as confident that marking functions calling other functions as `inline` works similarly to very simple functions, so I used `inline` over `inline(always)`.
nicopap
added
C-Performance
doonv
approved these changes
Jan 13, 2024
mockersf
approved these changes
Jan 13, 2024
mockersf
added
the
S-Ready-For-Final-Review
|
would be nice to see numbers on this, but I'm not sure we have an benchmark or a stress test that would show it |
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
In #11330 I found out that
Parent::getdidn't get inlined, even with LTO on!This means that just to access a field, we have an instruction cache invalidation, we will move some registers to the stack, will jump to new instructions, move the field into a register, then do the same dance in the other direction to go back to the call site.
Solution
Mark trivial functions as
#[inline].inline(always)may increase compilation time proportional to how many time the function is called and the size of the function marked withinline. Since we mark asinlinefunctions that consists in a single instruction, the cost is absolutely negligible.I also took the opportunity to
inlineother functions. I'm not as confident that marking functions calling other functions asinlineworks similarly to very simple functions, so I usedinlineoverinline(always), which doesn't have the same downsides asinline(always).More information on inlining in rust: https://nnethercote.github.io/perf-book/inlining.html