From 70a2a44f421377c920c2757b0d41c1949edc7363 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 15 Mar 2024 18:09:05 +0900 Subject: [PATCH] cli: ensure revset bench does not include cost of short-prefixes computation Spotted while moving revset::optimize() around. Since we don't include the parsing cost of the target expression, we shouldn't include parsing/evaluation cost of the short-prefixes either. The IdPrefixContext is currently populated by WorkspaceCommandHelper::new(), but it's hard to tell. --- cli/src/commands/bench.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cli/src/commands/bench.rs b/cli/src/commands/bench.rs index 865dd8cca..53b9ffe60 100644 --- a/cli/src/commands/bench.rs +++ b/cli/src/commands/bench.rs @@ -15,6 +15,7 @@ use std::fmt::Debug; use std::io; use std::io::Write as _; +use std::rc::Rc; use std::time::Instant; use clap::Subcommand; @@ -22,6 +23,7 @@ use criterion::measurement::Measurement; use criterion::{BatchSize, BenchmarkGroup, BenchmarkId, Criterion}; use jj_lib::object_id::HexPrefix; use jj_lib::repo::Repo; +use jj_lib::revset::{DefaultSymbolResolver, RevsetExpression}; use crate::cli_util::{CommandHelper, WorkspaceCommandHelper}; use crate::command_error::CommandError; @@ -204,12 +206,15 @@ fn bench_revset( writeln!(ui.stderr(), "----------Testing revset: {revset}----------")?; let expression = workspace_command.parse_revset(revset)?; // Time both evaluation and iteration. - let routine = |workspace_command: &WorkspaceCommandHelper, expression| { - workspace_command - .evaluate_revset(expression) - .unwrap() - .iter() - .count() + let routine = |workspace_command: &WorkspaceCommandHelper, expression: Rc| { + // Evaluate the expression without parsing/evaluating short-prefixes. + let repo = workspace_command.repo().as_ref(); + let symbol_resolver = DefaultSymbolResolver::new(repo); + let resolved = expression + .resolve_user_expression(repo, &symbol_resolver) + .unwrap(); + let revset = resolved.evaluate(repo).unwrap(); + revset.iter().count() }; let before = Instant::now(); let result = routine(workspace_command, expression.clone());