From 12a7607bac7cb9637f6ef90e894121129efca30b Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Mon, 26 Aug 2024 10:28:48 +0000 Subject: [PATCH] perf(codegen): inline `Codegen::print_list` (#5221) 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. --- crates/oxc_codegen/src/lib.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/oxc_codegen/src/lib.rs b/crates/oxc_codegen/src/lib.rs index ff47797a5932e..5f254ce2ab35d 100644 --- a/crates/oxc_codegen/src/lib.rs +++ b/crates/oxc_codegen/src/lib.rs @@ -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. + // fn print_list(&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); } }