From a03e9c41a377524be82e477af295b49a4cc1d599 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 29 Mar 2023 13:27:57 +0900
Subject: [PATCH] cli: run revset bench with fresh (but index-preloaded) repo
 instance
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some benchmark numbers in the following order:
- original
- fresh repo, BatchSize::SmallInput
- fresh but index-preloaded repo, BatchSize::SmallInput
- fresh but index-preloaded repo, BatchSize::LargeInput

    % cargo run --release --features bench -- bench revset ':main'
    revsets/:main           time:   [271.49 µs 271.74 µs 272.07 µs]
    revsets/:main           time:   [754.17 µs 758.22 µs 764.02 µs]
    revsets/:main           time:   [367.11 µs 372.65 µs 381.99 µs]
    revsets/:main           time:   [341.76 µs 342.98 µs 344.35 µs]

    % cargo run --release --features bench -- bench revset 'author(martinvonz)'
    revsets/author(martinvonz)
                            time:   [767.43 µs 770.52 µs 775.59 µs]
    revsets/author(martinvonz)
                            time:   [31.960 ms 31.984 ms 32.011 ms]
    revsets/author(martinvonz)
                            time:   [31.478 ms 31.538 ms 31.615 ms]
    revsets/author(martinvonz)
                            time:   [31.503 ms 31.526 ms 31.550 ms]

I think the fresh but index-preloaded repo is close to the practical
evaluation environment. With BatchSize::SmallInput, it appears to consume
~600MB (RES) memory (compared to ~50MB in LargeInput.) I don't think that's
huge, but it might affect cache usage, so I chose LargeInput.

https://docs.rs/criterion/latest/criterion/enum.BatchSize.html
---
 src/commands/bench.rs | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/commands/bench.rs b/src/commands/bench.rs
index bb2f14818..cbf3f88fa 100644
--- a/src/commands/bench.rs
+++ b/src/commands/bench.rs
@@ -18,7 +18,7 @@ use std::time::Instant;
 
 use clap::Subcommand;
 use criterion::measurement::Measurement;
-use criterion::{BenchmarkGroup, BenchmarkId, Criterion};
+use criterion::{BatchSize, BenchmarkGroup, BenchmarkId, Criterion};
 use jujutsu_lib::index::HexPrefix;
 use jujutsu_lib::repo::Repo;
 
@@ -146,6 +146,7 @@ pub(crate) fn cmd_bench(
             let mut group = criterion.benchmark_group("revsets");
             bench_revset(
                 ui,
+                command,
                 &workspace_command,
                 &mut group,
                 &command_matches.revisions,
@@ -165,7 +166,7 @@ pub(crate) fn cmd_bench(
                 if revset.starts_with('#') || revset.is_empty() {
                     continue;
                 }
-                bench_revset(ui, &workspace_command, &mut group, revset)?;
+                bench_revset(ui, command, &workspace_command, &mut group, revset)?;
             }
             // Neither of these seem to report anything...
             group.finish();
@@ -177,16 +178,15 @@ pub(crate) fn cmd_bench(
 
 fn bench_revset<M: Measurement>(
     ui: &mut Ui,
+    command: &CommandHelper,
     workspace_command: &WorkspaceCommandHelper,
     group: &mut BenchmarkGroup<M>,
     revset: &str,
 ) -> Result<(), CommandError> {
     writeln!(ui, "----------Testing revset: {revset}----------\n")?;
     let expression = workspace_command.parse_revset(revset)?;
-    // Time both evaluation and iteration. Note that we don't clear caches (such as
-    // commit objects in `Store`) between each run (`criterion`
-    // doesn't seem to support that).
-    let routine = |expression| {
+    // Time both evaluation and iteration.
+    let routine = |workspace_command: &WorkspaceCommandHelper, expression| {
         workspace_command
             .evaluate_revset(expression)
             .unwrap()
@@ -194,7 +194,7 @@ fn bench_revset<M: Measurement>(
             .count()
     };
     let before = Instant::now();
-    let result = routine(expression.clone());
+    let result = routine(workspace_command, expression.clone());
     let after = Instant::now();
     writeln!(
         ui,
@@ -206,7 +206,20 @@ fn bench_revset<M: Measurement>(
         BenchmarkId::from_parameter(revset),
         &expression,
         |bencher, expression| {
-            bencher.iter(|| routine(expression.clone()));
+            bencher.iter_batched(
+                // Reload repo and backend store to clear caches (such as commit objects
+                // in `Store`), but preload index since it's more likely to be loaded
+                // by preceding operation. `repo.reload_at()` isn't enough to clear
+                // store cache.
+                || {
+                    let workspace_command = command.workspace_helper_no_snapshot(ui).unwrap();
+                    workspace_command.repo().readonly_index();
+                    workspace_command
+                },
+                |workspace_command| routine(&workspace_command, expression.clone()),
+                // Index-preloaded repo may consume a fair amount of memory
+                BatchSize::LargeInput,
+            );
         },
     );
     Ok(())