Skip to content

Conversation

@therealprof
Copy link
Contributor

…red harmful

Signed-off-by: Daniel Egger daniel@eggers-club.de

…red harmful Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@japaric
Copy link
Member

japaric commented Sep 19, 2017

("being considered harmful" is not a very descriptive commit message / PR title, IMO.)

For posteriority, could you please add a comment (or a link) with the rationale about the change to this PR?

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit 73cd404 has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit 73cd404 with merge 895722b...

japaric pushed a commit that referenced this pull request Sep 20, 2017
Replace #[inline(always)] by #[inline] due to the later being conside… …red harmful Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@therealprof
Copy link
Contributor Author

Well, according to https://rust-lang-nursery.github.io/rust-clippy/master/index.html#inline_always it is considered harmful: It increases compile time and may increase size as well as decrease performance. I'm afraid I can't come up with a better label than "considered harmful".

@homunkulus
Copy link
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: japaric
Pushing 895722b to master...

@homunkulus homunkulus merged commit 73cd404 into rust-embedded:master Sep 20, 2017
@japaric japaric mentioned this pull request Sep 20, 2017
@therealprof therealprof deleted the no_always_inline branch September 23, 2017 21:54
therealprof added a commit that referenced this pull request Jul 20, 2019
Turns out this @therealprof dude was wrong in blindly following general consensus that #[inline(always)] was a bad idea. Revisiting the topic showed that just #[inline] is not enough to get even very trivial functions inlined in dev mode which causes a ton of bloat and a lot of debugging fun due to many emitted extra functions without any value for the developer. Usual binary reductions by this change are in the area of 10-15% which is a lot given that even a simply blinky on Cortex-M0 is several kB already. E.g. before: 0.5% 100.0% 4.0KiB .text section size, the file size is 734.1KiB after: 0.5% 100.0% 3.7KiB .text section size, the file size is 714.5KiB Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@therealprof therealprof mentioned this pull request Jul 20, 2019
bors bot added a commit that referenced this pull request Jul 21, 2019
324: improve dev builds r=ryankurte a=therealprof This PR basically reverts #141 and plays with inline markers for some large dev build binary size gains Co-authored-by: Daniel Egger <daniel@eggers-club.de>
therealprof added a commit that referenced this pull request Jul 21, 2019
Turns out this @therealprof dude was wrong in blindly following general consensus that #[inline(always)] was a bad idea. Revisiting the topic showed that just #[inline] is not enough to get even very trivial functions inlined in dev mode which causes a ton of bloat and a lot of debugging fun due to many emitted extra functions without any value for the developer. Usual binary reductions by this change are in the area of 10-15% which is a lot given that even a simply blinky on Cortex-M0 is several kB already. E.g. before: 0.5% 100.0% 4.0KiB .text section size, the file size is 734.1KiB after: 0.5% 100.0% 3.7KiB .text section size, the file size is 714.5KiB Signed-off-by: Daniel Egger <daniel@eggers-club.de>
bors bot added a commit that referenced this pull request Jul 21, 2019
324: improve dev builds r=ryankurte a=therealprof This PR basically reverts #141 and plays with inline markers for some large dev build binary size gains Co-authored-by: Daniel Egger <daniel@eggers-club.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants