From 6bc18e15e0b7985ecbb1edf099af88e43a44333c Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Mon, 22 Apr 2024 02:03:26 +0100 Subject: [PATCH] refactor(bench): reuse allocator in parser + lexer benchmarks (#3053) Re-use allocator in parser + lexer benchmarks. I believe this is the recommended usage when parsing a bunch of files - to re-use one allocator rather than create a fresh one for each run, so it makes sense to me that this is what the benchmark should measure. Doesn't show much difference on CodSpeed because it only runs the benchmark once, and it treats allocations as free anyway. But I imagine the difference may show up a bit more in a standard criterion benchmark. --- crates/oxc_allocator/src/lib.rs | 11 ++++++++++- tasks/benchmark/benches/lexer.rs | 12 ++++++------ tasks/benchmark/benches/parser.rs | 14 +++++++------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/crates/oxc_allocator/src/lib.rs b/crates/oxc_allocator/src/lib.rs index da6f1c14b..33e738d02 100644 --- a/crates/oxc_allocator/src/lib.rs +++ b/crates/oxc_allocator/src/lib.rs @@ -1,4 +1,7 @@ -use std::{convert::From, ops::Deref}; +use std::{ + convert::From, + ops::{Deref, DerefMut}, +}; mod arena; @@ -24,6 +27,12 @@ impl Deref for Allocator { } } +impl DerefMut for Allocator { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.bump + } +} + #[cfg(test)] mod test { use std::ops::Deref; diff --git a/tasks/benchmark/benches/lexer.rs b/tasks/benchmark/benches/lexer.rs index 6d2284767..7a95125f6 100644 --- a/tasks/benchmark/benches/lexer.rs +++ b/tasks/benchmark/benches/lexer.rs @@ -27,14 +27,14 @@ fn bench_lexer(criterion: &mut Criterion) { BenchmarkId::from_parameter(&file.file_name), &file.source_text, |b, source_text| { - b.iter_with_large_drop(|| { - // Include the allocator drop time to make time measurement consistent. - // Otherwise the allocator will allocate huge memory chunks (by power of two) from the - // system allocator, which makes time measurement unequal during long runs. - let allocator = Allocator::default(); + // Do not include initializing allocator in benchmark. + // User code would likely reuse the same allocator over and over to parse multiple files, + // so we do the same here. + let mut allocator = Allocator::default(); + b.iter(|| { let mut lexer = Lexer::new_for_benchmarks(&allocator, source_text, source_type); while lexer.next_token().kind != Kind::Eof {} - allocator + allocator.reset(); }); }, ); diff --git a/tasks/benchmark/benches/parser.rs b/tasks/benchmark/benches/parser.rs index db0aaa1f1..05418ee2f 100644 --- a/tasks/benchmark/benches/parser.rs +++ b/tasks/benchmark/benches/parser.rs @@ -12,13 +12,13 @@ fn bench_parser(criterion: &mut Criterion) { BenchmarkId::from_parameter(&file.file_name), &file.source_text, |b, source_text| { - b.iter_with_large_drop(|| { - // Include the allocator drop time to make time measurement consistent. - // Otherwise the allocator will allocate huge memory chunks (by power of two) from the - // system allocator, which makes time measurement unequal during long runs. - let allocator = Allocator::default(); - _ = Parser::new(&allocator, source_text, source_type).parse(); - allocator + // Do not include initializing allocator in benchmark. + // User code would likely reuse the same allocator over and over to parse multiple files, + // so we do the same here. + let mut allocator = Allocator::default(); + b.iter(|| { + Parser::new(&allocator, source_text, source_type).parse(); + allocator.reset(); }); }, );