Add NotebookIndex to the cache

[dhruvmanila]

Summary

This PR updates the FileCache to include an optional NotebookIndex to support caching for Jupyter Notebooks.

We only require the index to compute the diagnostics and thus we don't really need to store the entire Notebook on the Diagnostics struct. This means we only need the index to be stored in the cache to reconstruct the Diagnostics.

Test Plan

Update an existing test case to run over the fixtures under ruff_notebook crate where there are multiple Jupyter Notebook.

Locally, the following commands were run in order:

1. Remove the cache: rm -rf .ruff_cache
2. Run without cache: cargo run --bin ruff -- check --isolated crates/ruff_notebook/resources/test/fixtures/jupyter/unused_variable.ipynb --no-cache
3. Run with cache: cargo run --bin ruff -- check --isolated crates/ruff_notebook/resources/test/fixtures/jupyter/unused_variable.ipynb
4. Check whether the .ruff_cache directory was created or not
5. Run with cache again and verify: cargo run --bin ruff -- check --isolated crates/ruff_notebook/resources/test/fixtures/jupyter/unused_variable.ipynb

fixes: #6671

[github-actions]

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

[dhruvmanila]

Current dependencies on/for this PR:

• main
  • PR Add NotebookIndex to the cache #6863 👈

This comment was auto-generated by Graphite.

[MichaReiser]

Does the cached output now match the uncached output? How does the performance between cached and uncached notebooks compare?

[MichaReiser]

Suggested change

	/// Notebook index if this file is a Jupyter Notebook.
	/// Notebook index if this file is a IPython Notebook.

[dhruvmanila]

I think the notebook will still be termed as "Jupyter Notebook". It's the escape commands which are termed with the "Ipy" prefix.

[dhruvmanila]

Does the cached output now match the uncached output?

Yes, I've expanded the existing test case to test against the fixtures in ruff_notebook crate which contains a number of notebooks. I've also tested locally using the mentioned steps in the PR description and the output match.

How does the performance between cached and uncached notebooks compare?

Size

With a test directory containing a few repositories with Jupyter notebooks:

$ tokei ../test-repos/notebooks -t "Jupyter Notebooks"
===============================================================================
 Language            Files        Lines         Code     Comments       Blanks
===============================================================================
 Jupyter Notebooks    1221            0            0            0            0
 |- Markdown           272        14168          422        10048         3698
 |- Python             273        47807        37056         4219         6532
 (Total)                          61975        37478        14267        10230
===============================================================================
 Total                1221            0            0            0            0
===============================================================================

The disk size has increased because of the addition of notebook index stored in the cache:

$ du -h -d 1 ../cache-test                      
 12M	../cache-test/no-notebook-index
 14M	../cache-test/with-notebook-index

Performace

With the following ruff.toml:

include = ["*.ipynb"]

Earlier, the output of a cached run on Jupyter Notebooks were incorrect because the cell information was not stored in the cache. With that in mind, the benchmarks are as follows:

1. Uncached vs Cached runs on Jupyter Notebooks only file set:

$ hyperfine --warmup 5 \
  "./target/release/ruff check -e --config ./ruff.toml --no-cache --select=ALL ../test-repos/notebooks" \
  "./target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/notebook-index --select=ALL ../test-repos/notebooks"
Benchmark 1: ./target/release/ruff check -e --config ./ruff.toml --no-cache --select=ALL ../test-repos/notebooks
  Time (mean ± σ):      1.127 s ±  0.073 s    [User: 6.342 s, System: 0.391 s]
  Range (min … max):    1.018 s …  1.246 s    10 runs
 
Benchmark 2: ./target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/notebook-index --select=ALL ../test-repos/notebooks
  Time (mean ± σ):     192.9 ms ±   3.8 ms    [User: 277.7 ms, System: 138.7 ms]
  Range (min … max):   185.8 ms … 198.9 ms    15 runs
 
Summary
  ./target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/notebook-index --select=ALL ../test-repos/notebooks ran
    5.84 ± 0.40 times faster than ./target/release/ruff check -e --config ./ruff.toml --no-cache --select=ALL ../test-repos/notebooks

The numbers are in line with non-Jupyter Notebook dataset

2. Cached with and without NotebookIndex in the cache:

$ hyperfine --warmup 5 \
  "./../ruff/target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/ruff-old --select=ALL ../test-repos/notebooks" \
  "./target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/ruff-new --select=ALL ../test-repos/notebooks"      
Benchmark 1: ./../ruff/target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/ruff-old --select=ALL ../test-repos/notebooks
  Time (mean ± σ):     181.6 ms ±   3.6 ms    [User: 261.9 ms, System: 134.0 ms]
  Range (min … max):   177.5 ms … 188.5 ms    16 runs
 
Benchmark 2: ./target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/ruff-new --select=ALL ../test-repos/notebooks
  Time (mean ± σ):     188.6 ms ±   4.3 ms    [User: 276.8 ms, System: 134.8 ms]
  Range (min … max):   181.7 ms … 197.1 ms    15 runs
 
Summary
  ./../ruff/target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/ruff-old --select=ALL ../test-repos/notebooks ran
    1.04 ± 0.03 times faster than ./target/release/ruff check -e --config ./ruff.toml --cache-dir ../cache-test/ruff-new --select=ALL ../test-repos/notebooks

Here, we can see that the performance has gone down a bit because of the new NotebookIndex which needs to be cloned from the cache to be used on Diagnostics. Both FileCache and Diagnostics owns the NotebookIndex which is the reason for cloning.