From 6f8bb1cebf817c8b0bfab4be954a9023643ec2d1 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Mon, 16 Dec 2024 14:01:01 +0000 Subject: [PATCH] ci(benchmarks): simplify benchmarks (#7939) Pure refactor. Use `bench_function` instead of `bench_with_input` and just borrow data from outside closure. This shortens the code and (I think) makes it easier to read. --- .../benches/isolated_declarations.rs | 3 +- tasks/benchmark/benches/lexer.rs | 28 +++++++------- tasks/benchmark/benches/linter.rs | 37 +++++++++---------- tasks/benchmark/benches/parser.rs | 36 +++++++++--------- tasks/benchmark/benches/prettier.rs | 23 +++++------- tasks/benchmark/benches/semantic.rs | 32 ++++++++-------- 6 files changed, 74 insertions(+), 85 deletions(-) diff --git a/tasks/benchmark/benches/isolated_declarations.rs b/tasks/benchmark/benches/isolated_declarations.rs index 91d02286fc43c..6f638b9877b60 100644 --- a/tasks/benchmark/benches/isolated_declarations.rs +++ b/tasks/benchmark/benches/isolated_declarations.rs @@ -13,9 +13,10 @@ fn bench_isolated_declarations(criterion: &mut Criterion) { ); let id = BenchmarkId::from_parameter(&file.file_name); + let source_text = file.source_text.as_str(); let source_type = SourceType::from_path(&file.file_name).unwrap(); - group.bench_with_input(id, &file.source_text, |b, source_text| { + group.bench_function(id, |b| { b.iter_with_large_drop(|| { let allocator = Allocator::default(); let ParserReturn { program, .. } = diff --git a/tasks/benchmark/benches/lexer.rs b/tasks/benchmark/benches/lexer.rs index e0aebdefdcf17..234da608bf81f 100644 --- a/tasks/benchmark/benches/lexer.rs +++ b/tasks/benchmark/benches/lexer.rs @@ -23,22 +23,20 @@ fn bench_lexer(criterion: &mut Criterion) { .collect::>(); for file in files { + let id = BenchmarkId::from_parameter(&file.file_name); + let source_text = file.source_text.as_str(); let source_type = SourceType::from_path(&file.file_name).unwrap(); - group.bench_with_input( - BenchmarkId::from_parameter(&file.file_name), - &file.source_text, - |b, source_text| { - // 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.reset(); - }); - }, - ); + group.bench_function(id, |b| { + // 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.reset(); + }); + }); } group.finish(); } diff --git a/tasks/benchmark/benches/linter.rs b/tasks/benchmark/benches/linter.rs index 9b51eef628f36..d98ac22a5514a 100644 --- a/tasks/benchmark/benches/linter.rs +++ b/tasks/benchmark/benches/linter.rs @@ -24,27 +24,24 @@ fn bench_linter(criterion: &mut Criterion) { } for file in test_files { + let id = BenchmarkId::from_parameter(&file.file_name); + let source_text = file.source_text.as_str(); let source_type = SourceType::from_path(&file.file_name).unwrap(); - group.bench_with_input( - BenchmarkId::from_parameter(&file.file_name), - &file.source_text, - |b, source_text| { - let allocator = Allocator::default(); - let ret = Parser::new(&allocator, source_text, source_type).parse(); - let path = Path::new(""); - let semantic_ret = SemanticBuilder::new() - .with_build_jsdoc(true) - .with_scope_tree_child_ids(true) - .with_cfg(true) - .build(&ret.program); - let semantic = semantic_ret.semantic; - let module_record = - Arc::new(ModuleRecord::new(path, &ret.module_record, &semantic)); - let semantic = Rc::new(semantic); - let linter = LinterBuilder::all().with_fix(FixKind::All).build(); - b.iter(|| linter.run(path, Rc::clone(&semantic), Arc::clone(&module_record))); - }, - ); + group.bench_function(id, |b| { + let allocator = Allocator::default(); + let ret = Parser::new(&allocator, source_text, source_type).parse(); + let path = Path::new(""); + let semantic_ret = SemanticBuilder::new() + .with_build_jsdoc(true) + .with_scope_tree_child_ids(true) + .with_cfg(true) + .build(&ret.program); + let semantic = semantic_ret.semantic; + let module_record = Arc::new(ModuleRecord::new(path, &ret.module_record, &semantic)); + let semantic = Rc::new(semantic); + let linter = LinterBuilder::all().with_fix(FixKind::All).build(); + b.iter(|| linter.run(path, Rc::clone(&semantic), Arc::clone(&module_record))); + }); } group.finish(); } diff --git a/tasks/benchmark/benches/parser.rs b/tasks/benchmark/benches/parser.rs index f0f013c4c589b..5ede362633e83 100644 --- a/tasks/benchmark/benches/parser.rs +++ b/tasks/benchmark/benches/parser.rs @@ -7,26 +7,24 @@ use oxc_tasks_common::TestFiles; fn bench_parser(criterion: &mut Criterion) { let mut group = criterion.benchmark_group("parser"); for file in TestFiles::complicated().files() { + let id = BenchmarkId::from_parameter(&file.file_name); + let source_text = file.source_text.as_str(); let source_type = SourceType::from_path(&file.file_name).unwrap(); - group.bench_with_input( - BenchmarkId::from_parameter(&file.file_name), - &file.source_text, - |b, source_text| { - // 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) - .with_options(ParseOptions { - parse_regular_expression: true, - ..ParseOptions::default() - }) - .parse(); - allocator.reset(); - }); - }, - ); + group.bench_function(id, |b| { + // 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) + .with_options(ParseOptions { + parse_regular_expression: true, + ..ParseOptions::default() + }) + .parse(); + allocator.reset(); + }); + }); } group.finish(); } diff --git a/tasks/benchmark/benches/prettier.rs b/tasks/benchmark/benches/prettier.rs index 0c0961b36f1ea..99ad05811ba7c 100644 --- a/tasks/benchmark/benches/prettier.rs +++ b/tasks/benchmark/benches/prettier.rs @@ -8,20 +8,17 @@ use oxc_tasks_common::TestFiles; fn bench_prettier(criterion: &mut Criterion) { let mut group = criterion.benchmark_group("prettier"); for file in TestFiles::minimal().files() { + let id = BenchmarkId::from_parameter(&file.file_name); + let source_text = file.source_text.as_str(); let source_type = SourceType::from_path(&file.file_name).unwrap(); - group.bench_with_input( - BenchmarkId::from_parameter(&file.file_name), - &file.source_text, - |b, source_text| { - b.iter(|| { - let allocator1 = Allocator::default(); - let allocator2 = Allocator::default(); - let ret = Parser::new(&allocator1, source_text, source_type).parse(); - let _ = - Prettier::new(&allocator2, PrettierOptions::default()).build(&ret.program); - }); - }, - ); + group.bench_function(id, |b| { + b.iter(|| { + let allocator1 = Allocator::default(); + let allocator2 = Allocator::default(); + let ret = Parser::new(&allocator1, source_text, source_type).parse(); + let _ = Prettier::new(&allocator2, PrettierOptions::default()).build(&ret.program); + }); + }); } group.finish(); } diff --git a/tasks/benchmark/benches/semantic.rs b/tasks/benchmark/benches/semantic.rs index 96422cdc9da5a..37583f02469e7 100644 --- a/tasks/benchmark/benches/semantic.rs +++ b/tasks/benchmark/benches/semantic.rs @@ -8,24 +8,22 @@ use oxc_tasks_common::TestFiles; fn bench_semantic(criterion: &mut Criterion) { let mut group = criterion.benchmark_group("semantic"); for file in TestFiles::complicated().files() { + let id = BenchmarkId::from_parameter(&file.file_name); + let source_text = file.source_text.as_str(); let source_type = SourceType::from_path(&file.file_name).unwrap(); - group.bench_with_input( - BenchmarkId::from_parameter(&file.file_name), - &file.source_text, - |b, source_text| { - let allocator = Allocator::default(); - let ret = Parser::new(&allocator, source_text, source_type).parse(); - b.iter_with_large_drop(|| { - // We drop `Semantic` inside this closure as drop time is part of cost of using this API. - // We return `error`s to be dropped outside of the measured section, as usually - // code would have no errors. One of our benchmarks `cal.com.tsx` has a lot of errors, - // but that's atypical, so don't want to include it in benchmark time. - let ret = SemanticBuilder::new().with_build_jsdoc(true).build(&ret.program); - let ret = black_box(ret); - ret.errors - }); - }, - ); + group.bench_function(id, |b| { + let allocator = Allocator::default(); + let ret = Parser::new(&allocator, source_text, source_type).parse(); + b.iter_with_large_drop(|| { + // We drop `Semantic` inside this closure as drop time is part of cost of using this API. + // We return `error`s to be dropped outside of the measured section, as usually + // code would have no errors. One of our benchmarks `cal.com.tsx` has a lot of errors, + // but that's atypical, so don't want to include it in benchmark time. + let ret = SemanticBuilder::new().with_build_jsdoc(true).build(&ret.program); + let ret = black_box(ret); + ret.errors + }); + }); } group.finish(); }