Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

without_extras is slow for lerobot #10430

Open
konstin opened this issue Jan 9, 2025 · 3 comments
Open

without_extras is slow for lerobot #10430

konstin opened this issue Jan 9, 2025 · 3 comments
Assignees
Labels
performance Potential performance improvement

Comments

@konstin
Copy link
Member

konstin commented Jan 9, 2025

Given this pyproject.toml, we spend ~38% of resolver thread time in without_extras

Image

Hot code paths:

// In the branches, we "sort" the preferences by marker-matching through an iterator that
// first has the matching half and then the mismatching half.
let preferences_match = preferences
.get(package_name)
.filter(|(marker, _index, _version)| env.included_by_marker(marker.pep508()));

MarkerTree(INTERNER.lock().without_extras(self.0))

Profiling command, on ubuntu 24.04:

cargo build --profile profiling
rm -f uv.lock
samply record --rate 20000 target/profiling/uv lock -p 3.12

We should improve the performance for lerobot.

@konstin konstin added the performance Potential performance improvement label Jan 9, 2025
@BurntSushi
Copy link
Member

One simplistic idea is to create a PEP 508 marker once and store it on UniversalMarker. Then the pep508 method can just return it instead of creating it.

I think the main challenge there is just making sure it gets updated as appropriate since a UniversalMarker has &mut methods.

This strategy is predicated on the idea that marker mutation is much rarer than calling m.pep508(). I think that's probably true, but I'm not certain.

@BurntSushi
Copy link
Member

(It might also be a good time to get rid of the &mut methods on all of the marker types completely and just double down on its Copy impl.)

@charliermarsh
Copy link
Member

We can maybe also optimize the caller here? It looks like we call marker.pep508() twice for each package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Potential performance improvement
Projects
None yet
Development

No branches or pull requests

3 participants