Skip to content

Commit

Permalink
perf(codegen): inline Codegen::print_list (#5221)
Browse files Browse the repository at this point in the history
Revert #5192 and add a comment that it's not a perf gain.

This was really surprising to me, but the benchmarks do demonstrate it.

Please see the benchmarks commit-by-commit on this PR. Adding `#[inline]` to the function does give +1% gain, but it's no better than it was before #5192. So I think preferable to just revert to the simpler original.

I think likely explanation is that the compiler is already performing this optimization itself. And if it does it itself, then it understands the code better, and can then make better decisions about inlining.

https://godbolt.org/z/xzhWWeMoe seems to demonstrate this - there are 2 calls to `Item::gen` in the generated assembly, so it has split the loop into 2.
  • Loading branch information
overlookmotel committed Aug 26, 2024
1 parent 1686920 commit 12a7607
Showing 1 changed file with 18 additions and 6 deletions.
24 changes: 18 additions & 6 deletions crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,13 +411,25 @@ impl<'a> Codegen<'a> {
self.needs_semicolon = false;
}

// We tried optimizing this to move the `index != 0` check out of the loop:
// ```
// let mut iter = items.iter();
// let Some(item) = iter.next() else { return };
// item.gen(self, ctx);
// for item in iter {
// self.print_comma();
// self.print_soft_space();
// item.gen(self, ctx);
// }
// ```
// But it turned out this was actually a bit slower.
// <https://github.com/oxc-project/oxc/pull/5221>
fn print_list<T: Gen>(&mut self, items: &[T], ctx: Context) {
let mut iter = items.iter();
let Some(item) = iter.next() else { return };
item.gen(self, ctx);
for item in iter {
self.print_comma();
self.print_soft_space();
for (index, item) in items.iter().enumerate() {
if index != 0 {
self.print_comma();
self.print_soft_space();
}
item.gen(self, ctx);
}
}
Expand Down

0 comments on commit 12a7607

Please sign in to comment.