Remove unnecessary #[inline] hints
#3867
Open
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
daxpedda
requested review from
jackpot51,
notgull,
kchibisov,
madsmtm and
MarijnS95
as code owners
|
Fwiw, in So I hope you're leaving public functions that call public functions/constructors in place? |
MarijnS95
reviewed
Aug 14, 2024
Comment on lines
-160
to
159
| #[inline] | ||
| pub fn cast<X: Pixel>(&self) -> LogicalUnit<X> { | ||
| LogicalUnit(self.0.cast()) |
There was a problem hiding this comment.
Small public function calling public code? All of these, it seems like?
There was a problem hiding this comment.
Emphasis on non-generic.
There was a problem hiding this comment.
Let's wake up and try reading this again after a few hours 🙈.
Yes! |
madsmtm
approved these changes
Aug 14, 2024
kchibisov
approved these changes
Aug 15, 2024
kchibisov
requested changes
Aug 23, 2024
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.
As discussed in the last meeting, this PR removes unnecessary
#[inline]hints, which hurt compile times and sometimes even performance.To summarize:
#[inline]to generic functions is not recommended.#[inline]to non-public functions is not recommended.#[inline]to public functions because they only call a private function is not recommended, unless that private function is small and is annotated with#[inline]as well.#[inline]should only be used for small non-generic public functions or after some benchmarking.See https://matklad.github.io/2021/07/09/inline-in-rust.html and rust-lang/hashbrown#119 (comment) for more detailed information and recommendations.