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

refactor: make solver generic #245

Merged
merged 10 commits into from
Jul 6, 2023
Merged

Conversation

baszalmstra
Copy link
Collaborator

@baszalmstra baszalmstra commented Jul 4, 2023

This refactors the code a little bit to make SolverTask and SolverBackend a bit more generic. The biggest change is that a SolverBackend can use any SolverTask where the available_packages are a container of which the elements are RepoDataRecords which can be borrowed.

The nice thing about this is that now its easier to create functions that are generic over the SolverBackend. This was required to make tests that are only used when certain backends are enabled without having to copy code per solver.

Additionally, I split the two solvers into separate modules so we don't get LibSolvRsSolver names but can use libsolv_rs::Solver instead.

Also added some benchmarks. On my machine:

libsolv libsolv-rs diff
python=3.9 13.19ms 14.69ms +1.5ms (11%)
tensorboard=2.1.1, grpc-cpp=1.39.1 1.78s 2.12s +0.3s (20%)

@baszalmstra
Copy link
Collaborator Author

Ah this is far from done. There are many more tests to fix.

@baszalmstra
Copy link
Collaborator Author

Good to go now! @wolfv

@wolfv
Copy link
Contributor

wolfv commented Jul 5, 2023

I think you need to use libsolv_rs for the feature?

@wolfv
Copy link
Contributor

wolfv commented Jul 5, 2023

Here are my measurements on macOS :)

     Running benches/bench.rs (/Users/wolfv/Programs/rattler/target/release/deps/bench-aeb2a05fd163e9ec)
Gnuplot not found, using plotters backend
solve python=3.9/libsolv-sys
                        time:   [6.7331 ms 6.7643 ms 6.8058 ms]
                        change: [-3.4709% -2.9452% -2.3833%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) low mild
  4 (4.00%) high mild
  3 (3.00%) high severe
solve python=3.9/libsolv_rs
                        time:   [5.3383 ms 5.3591 ms 5.3816 ms]
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) high mild
  2 (2.00%) high severe

Benchmarking solve tensorboard=2.1.1, grpc-cpp=1.39.1/libsolv-sys: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 45.8s, or reduce sample count to 10.
solve tensorboard=2.1.1, grpc-cpp=1.39.1/libsolv-sys
                        time:   [458.49 ms 460.34 ms 462.71 ms]
                        change: [-23.282% -22.949% -22.548%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) high mild
  5 (5.00%) high severe
Benchmarking solve tensorboard=2.1.1, grpc-cpp=1.39.1/libsolv_rs: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 95.3s, or reduce sample count to 10.
solve tensorboard=2.1.1, grpc-cpp=1.39.1/libsolv_rs
                        time:   [955.09 ms 956.73 ms 958.51 ms]
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

@baszalmstra
Copy link
Collaborator Author

@wolfv very interesting! In the simple case the port seems faster whereas in the larger case its much slower.

@wolfv
Copy link
Contributor

wolfv commented Jul 5, 2023

Yeah, but we need to take the measurements with a grain of salt because of laptop / cpu warmup etc... but it's great to have some benchmarks!

@baszalmstra
Copy link
Collaborator Author

@wolfv I think this is ready to go!

@wolfv wolfv merged commit 5b02600 into conda:main Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants