Lazy index

[zklaus]

Description

This was triggered by conda/conda-build#5154.
The goal is to introduce an Index API as a clean interface for the index, so as to allow well-defined interactions and avoid direct manipulation of Solver._index.
Currently, the index is handled as a simple dict, which is manipulated in various places to update with changed state.
This is particularly relevant in conda-build, where the index needs to be updated after the creation of new artifacts.
Introducing a standardized API makes this reloading more explicit.
The class Index also follows the spirit of PackageRecordList to avoid instantiating PackageRecords where not necessary, yielding both memory and time savings.

Additionally, this introduces memray into the testing facilities. With this PR, it becomes possible to use a pytest.mark.memray marker, which will make the test run in the linux-memray job (in addition to any other runs that may occur) under the observation of the pytest-memray plugin, which allows us to track memory usage.
Currently, this only leads to a little summary blob in the test logs; a future PR may expand upon this with persistent tracking of memory requirements similar to the benchmarking we do with codspeed.

Checklist - did you ...

• Add a file to the news directory (using the template) for the next release's release notes?
• Add / update necessary tests?
• Add / update outdated documentation?

[codspeed-hq]

CodSpeed Performance Report

Merging #13880 will not alter performance

_{Comparing zklaus:lazy-index (a4fa702) with main (eb45954)}

Summary

✅ 21 untouched benchmarks

[dholth]

How does this compare to

conda/conda/core/subdir_data.py

Line 92 in be5da40

class PackageRecordList(UserList):

[zklaus]

@dholth, I added a bit of explanation to the PR description. Basically, it is inspired by PackageRecordList and tries to bring the lazy PackageRecord instantiation from there (and the related SubdirData and friends) to the index.

Tracking memory usage and time with pytest-memray, the current dict approach uses 50s and

Allocation results for tests/core/test_index.py::test_get_index_lazy at the high watermark

         📦 Total memory allocated: 1.8GiB
         📏 Total allocations: 104
         📊 Histogram of allocation sizes: |█▇▂▇▁|
         🥇 Biggest allocating functions:
                - raw_decode:/opt/conda/lib/python3.12/json/decoder.py:353 -> 1.1GiB
                - __hash__:/workspaces/conda-workspace/conda/conda/models/records.py:300 -> 182.5MiB
                - __set__:/workspaces/conda-workspace/conda/conda/auxlib/entity.py:441 -> 124.0MiB
                - join:/workspaces/conda-workspace/conda/conda/common/url.py:315 -> 83.0MiB
                - _pkey:/workspaces/conda-workspace/conda/conda/models/records.py:293 -> 43.1MiB

and the new lazy approach 10s and

Allocation results for tests/core/test_index.py::test_get_index_lazy at the high watermark

         📦 Total memory allocated: 1.2GiB
         📏 Total allocations: 91
         📊 Histogram of allocation sizes: |▁█▂▁ |
         🥇 Biggest allocating functions:
                - raw_decode:/opt/conda/lib/python3.12/json/decoder.py:353 -> 822.1MiB
                - decode:<frozen codecs>:322 -> 243.6MiB
                - raw_decode:/opt/conda/lib/python3.12/json/decoder.py:353 -> 146.5MiB
                - join:/workspaces/conda-workspace/conda/conda/common/url.py:315 -> 5.0MiB
                - _process_raw_repodata:/workspaces/conda-workspace/conda/conda/core/subdir_data.py:489 -> 2.0MiB

i.e. a reduction by 40s and 600MB, though a bunch of integration tests are currently failing.

[dholth]

That's phenomenal.

[zklaus]

pre-commit.ci autofix

[jezdez]

This looks very promising, but IMO could go a little further in subsuming the supplement functions into Index instance methods (not static nor class methods).

The get_reduced_index could also use the Index internally instead of creating an ad-hoc dict instead.

[jezdez]

Suggested change

	if isinstance(prefix, PrefixData):
	elif isinstance(prefix, PrefixData):

[zklaus]

Functionally, it doesn't matter, of course, due to the return in the first if branch. Semantically, the first "if" deals with the index argument, the second if-else with the prefix argument, so on balance, I'd prefer to keep it as is. Does that work for you?

[jezdez]

Why isn't this passing main as well?

[zklaus]

This is to test the prepend feature, analogous to the other test_get_index_xxx_platform tests.

[jezdez]

As an example, this would be ideal to move into an instance method Index._supplement_prefix(prefix) and deprecate the function. Same for _supplement_index_with_cache and the other _supplement_* functions.

[zklaus]

Thanks for the quick review, @jezdez!

This looks very promising, but IMO could go a little further in subsuming the supplement functions into Index instance methods (not static nor class methods).

Dang, I already had the deprecations in place 😄

I removed them again because this is designed now as a drop-in replacement and it can be difficult to know what other code might rely on functions like these. For example, we have https://github.com/conda/conda-libmamba-solver/blob/1ac0aee96c4618c29ae2d2491fdc2ccd2b3305d4/conda_libmamba_solver/state.py#L186.

But if you are happy to remove them, then so am I.

We could even go further. Iterating an index is always expensive, particularly for large channels, so why not deprecate that? With the Index class in place we could to that by deprecating .__iter__, .keys, .values, .items.

The get_reduced_index could also use the Index internally instead of creating an ad-hoc dict instead.
Sure. Let me add that.

[jezdez]

Thanks for the quick review, @jezdez!

This looks very promising, but IMO could go a little further in subsuming the supplement functions into Index instance methods (not static nor class methods).

Dang, I already had the deprecations in place 😄

I removed them again because this is designed now as a drop-in replacement and it can be difficult to know what other code might rely on functions like these. For example, we have https://github.com/conda/conda-libmamba-solver/blob/1ac0aee96c4618c29ae2d2491fdc2ccd2b3305d4/conda_libmamba_solver/state.py#L186.

To be clear, we should still provide stub functions that are properly deprecated per CEP-9 that call Index behind the scenes. But for CLS specifically we maintain the API, so this is relatively painless by wrapping it into a try/except or similar techniques of inspecting conda's version and using a different import, or wrapping function. There are a few ways to do this.

But if you are happy to remove them, then so am I.

We could even go further. Iterating an index is always expensive, particularly for large channels, so why not deprecate that? With the Index class in place we could to that by deprecating .__iter__, .keys, .values, .items.

Migrating from dict with these methods to UserDict implies the methods remain, so not sure if deprecating the iterator API makes sense

The get_reduced_index could also use the Index internally instead of creating an ad-hoc dict instead.
Sure. Let me add that.

[dholth]

Suggested change

	* Add `conda.core.index.Index` as drop-in replacement of realized dictionary index. (#13880)
	* Add `conda.core.index.Index` as a faster drop-in replacement of realized dictionary index. (#13880)

It would make sense to mention why the replacement is better.

[zklaus]

pre-commit.ci autofix

[jaimergp]

I think this could be a property or a method. Doing this in initialization regardless the value of add_system might be expensive (e.g. the __cuda package has a non negligible overhead).

[zklaus]

To give a bit of an update: It took me some time last week to make this work with get_reduced_index, which uncovered some bugs and rough edges. Now it works, but as you can see from the codspeed comment above this results in some unacceptable slow down.
Figuring out why that is, it ultimately comes down to the combination of the solver doing a bunch of iterating over the packages, for example Resolve.__init__ has no less than 5 full iterations over the index, and the size of the lazy index. Since for iterating the lazy Index behaves exactly as the current index with a realized dictionary, this is of course much slower with the full index compared to the reduced index.
I spent some time to try and remove the iterating from the solver, but ultimately gave up on that because it is too much, out-of-scope for this PR, and possibly to invasive a change.
Let's try to add a reduction to the Index class instead.