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

Rust graph #166

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Rust graph #166

wants to merge 5 commits into from

Conversation

seddonym
Copy link
Owner

@seddonym seddonym commented Nov 1, 2024

Got it working, but it's very slow!

Things to try:

  • Replacing Module{ String } with Module { Arc<String> }.
  • String interning for module names.
  • Preallocating a size for certain hashmaps.
  • Rather than repeatedly cloning the whole Graph struct, could we just clone the inner imports StableGraph?
  • Could we avoid mutating the graph altogether when checking for imports? We brought in module squashing etc. to
    speed up the Python, but perhaps Rust is fast enough that we don't need to bother.

Copy link

codspeed-hq bot commented Dec 9, 2024

CodSpeed Performance Report

Merging #166 will degrade performances by 99.94%

Comparing rust-graph (43235d9) with master (6523dfe)

Summary

⚡ 4 improvements
❌ 5 regressions
✅ 2 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark master rust-graph Change
test_chain_found 67.1 µs 65,091.4 µs -99.9%
test_no_chain 39.9 µs 64,891.2 µs -99.94%
test_chains_found 128 ms 80.8 ms +58.34%
test_no_chains 128.7 ms 80.4 ms +60.08%
test_build_django_from_cache 224.9 ms 442.5 ms -49.17%
test_deep_layers_large_graph 1.5 s 5.9 s -74%
test_find_downstream_modules 2,489.4 ms 9 ms ×280
test_find_upstream_modules 1,828.1 ms 3.3 ms ×550
test_top_level_large_graph 784.1 ms 4,597.5 ms -82.94%

@seddonym seddonym force-pushed the rust-graph branch 9 times, most recently from aa72c92 to 25b711f Compare January 15, 2025 16:47
This is the main graph implemented in Rust - but it isn't plumbed into
Python yet.
This adds a wrapper around the Rust-graph in lib.rs which handles the
interface between Python and Rust. At the same time, it changes the
Python ImportGraph so it uses Rust instead.
This is faster, but not cryptographically safe. That shouldn't matter for our
purposes.
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.

1 participant