Skip to content

Conversation

@orxfun
Copy link
Owner

@orxfun orxfun commented Nov 22, 2025

Implements Send and Sync for NodeIdx.

Carries out a major refactoring for indices passed as values rather than references.

Using Copy semantics for accessing nodes simplifies the api; however, leads to a breaking change.

Fixes #177

Copy link
Contributor

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good to me, with some minor questions.
Might want to add #175 to the "fixes" list. Didnt see #176.

I am curious if there was any performance difference in the benches now that there is more copy and taking less references.

(i have not yet run it in my project)

/// let leaves: Vec<_> = n3.leaves::<PostOrder>().copied().collect();
/// assert_eq!(leaves, [209, 210, 211]);
/// ```
pub fn leaves_mut<T>(&'a mut self) -> impl Iterator<Item = &'a mut V::Item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No leaves_mut_with?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard this comment, didnt see #176.

@hasezoey
Copy link
Contributor

hasezoey commented Nov 23, 2025

As i was curious about performance changes, here are some test results:

Bench walk_iterator (as the others currently dont work correctly):

At 66363f4 (before #172):

Benchmarking walk_iterator/arbitrary_order_iter/65536: Collecting 100 samples in estimated 5.2929 s (1000 iterationswalk_iterator/arbitrary_order_iter/65536
                        time:   [5.2152 ms 5.2230 ms 5.2314 ms]
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe
Benchmarking walk_iterator/arbitrary_order_par_iter/65536: Collecting 100 samples in estimated 5.4640 s (1000 iteratwalk_iterator/arbitrary_order_par_iter/65536
                        time:   [5.4480 ms 5.4556 ms 5.4633 ms]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
Benchmarking walk_iterator/arbitrary_order_par_iter_with_rayon/65536: Collecting 100 samples in estimated 5.2808 s (walk_iterator/arbitrary_order_par_iter_with_rayon/65536
                        time:   [10.562 ms 10.584 ms 10.608 ms]
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) low mild
  1 (1.00%) high mild
  4 (4.00%) high severe
walk_iterator/walk::<Dfs>/65536
                        time:   [5.6522 ms 5.6570 ms 5.6616 ms]
Found 14 outliers among 100 measurements (14.00%)
  6 (6.00%) low severe
  5 (5.00%) high mild
  3 (3.00%) high severe
walk_iterator/walk_par::<Dfs>/65536
                        time:   [5.5807 ms 5.5885 ms 5.5965 ms]
Found 31 outliers among 100 measurements (31.00%)
  15 (15.00%) low severe
  5 (5.00%) low mild
  8 (8.00%) high mild
  3 (3.00%) high severe
walk_iterator/walk::<Bfs>/65536
                        time:   [5.7118 ms 5.7223 ms 5.7331 ms]
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild
walk_iterator/walk_par::<Bfs>/65536
                        time:   [6.3514 ms 6.3625 ms 6.3763 ms]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
walk_iterator/walk::<PostOrder>/65536
                        time:   [5.7890 ms 5.8009 ms 5.8126 ms]
Benchmarking walk_iterator/walk_par::<PostOrder>/65536: Collecting 100 samples in estimated 5.1667 s (900 iterationswalk_iterator/walk_par::<PostOrder>/65536
                        time:   [5.7187 ms 5.7238 ms 5.7295 ms]
Found 13 outliers among 100 measurements (13.00%)
  6 (6.00%) high mild
  7 (7.00%) high severe

At 796f6ff (current latest main):

Benchmarking walk_iterator/arbitrary_order_iter/65536: Collecting 100 samples in estimated 5.2764 s (1000 iterationswalk_iterator/arbitrary_order_iter/65536
                        time:   [5.2400 ms 5.2446 ms 5.2496 ms]
Found 12 outliers among 100 measurements (12.00%)
  6 (6.00%) high mild
  6 (6.00%) high severe
Benchmarking walk_iterator/arbitrary_order_par_iter/65536: Collecting 100 samples in estimated 5.3891 s (1000 iteratwalk_iterator/arbitrary_order_par_iter/65536
                        time:   [5.4407 ms 5.4522 ms 5.4640 ms]
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild
Benchmarking walk_iterator/arbitrary_order_par_iter_with_rayon/65536: Collecting 100 samples in estimated 5.2686 s (walk_iterator/arbitrary_order_par_iter_with_rayon/65536
                        time:   [10.523 ms 10.546 ms 10.575 ms]
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) low mild
  1 (1.00%) high mild
  4 (4.00%) high severe
walk_iterator/walk::<Dfs>/65536
                        time:   [5.5881 ms 5.6017 ms 5.6159 ms]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
walk_iterator/walk_par::<Dfs>/65536
                        time:   [5.5296 ms 5.5376 ms 5.5457 ms]
walk_iterator/walk::<Bfs>/65536
                        time:   [5.6825 ms 5.6861 ms 5.6902 ms]
Found 11 outliers among 100 measurements (11.00%)
  5 (5.00%) high mild
  6 (6.00%) high severe
walk_iterator/walk_par::<Bfs>/65536
                        time:   [6.3114 ms 6.3172 ms 6.3238 ms]
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) high mild
  5 (5.00%) high severe
walk_iterator/walk::<PostOrder>/65536
                        time:   [5.7837 ms 5.7927 ms 5.8019 ms]
Benchmarking walk_iterator/walk_par::<PostOrder>/65536: Collecting 100 samples in estimated 5.2899 s (900 iterationswalk_iterator/walk_par::<PostOrder>/65536
                        time:   [5.8156 ms 5.8291 ms 5.8462 ms]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
Compared against 66363f4:
walk_iterator/arbitrary_order_iter/65536
                        time:   [5.2400 ms 5.2446 ms 5.2499 ms]
                        change: [+0.2333% +0.4150% +0.5876%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 12 outliers among 100 measurements (12.00%)
  6 (6.00%) high mild
  6 (6.00%) high severe
walk_iterator/arbitrary_order_par_iter/65536
                        time:   [5.4408 ms 5.4522 ms 5.4639 ms]
                        change: [-0.3062% -0.0625% +0.1924%] (p = 0.64 > 0.05)
                        No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild
walk_iterator/arbitrary_order_par_iter_with_rayon/65536
                        time:   [10.523 ms 10.546 ms 10.575 ms]
                        change: [-0.6749% -0.3594% -0.0465%] (p = 0.03 < 0.05)
                        Change within noise threshold.
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) low mild
  1 (1.00%) high mild
  4 (4.00%) high severe
walk_iterator/walk::<Dfs>/65536
                        time:   [5.5881 ms 5.6017 ms 5.6156 ms]
                        change: [-1.2093% -0.9773% -0.7219%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
walk_iterator/walk_par::<Dfs>/65536
                        time:   [5.5297 ms 5.5376 ms 5.5458 ms]
                        change: [-1.1100% -0.9112% -0.7064%] (p = 0.00 < 0.05)
                        Change within noise threshold.
walk_iterator/walk::<Bfs>/65536
                        time:   [5.6825 ms 5.6861 ms 5.6902 ms]
                        change: [-0.8292% -0.6322% -0.4319%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 11 outliers among 100 measurements (11.00%)
  5 (5.00%) high mild
  6 (6.00%) high severe
walk_iterator/walk_par::<Bfs>/65536
                        time:   [6.3115 ms 6.3172 ms 6.3238 ms]
                        change: [-0.9405% -0.7121% -0.5060%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) high mild
  5 (5.00%) high severe
walk_iterator/walk::<PostOrder>/65536
                        time:   [5.7838 ms 5.7927 ms 5.8019 ms]
                        change: [-0.3941% -0.1422% +0.1116%] (p = 0.29 > 0.05)
                        No change in performance detected.
walk_iterator/walk_par::<PostOrder>/65536
                        time:   [5.8156 ms 5.8291 ms 5.8462 ms]
                        change: [+1.5708% +1.8389% +2.1326%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

At 7c8a447 (current latest commit in this PR):

Benchmarking walk_iterator/arbitrary_order_iter/65536: Collecting 100 samples in estimated 5.2542 s (1000 iterationswalk_iterator/arbitrary_order_iter/65536
                        time:   [5.2201 ms 5.2300 ms 5.2402 ms]
Benchmarking walk_iterator/arbitrary_order_par_iter/65536: Collecting 100 samples in estimated 5.3404 s (1000 iteratwalk_iterator/arbitrary_order_par_iter/65536
                        time:   [5.3812 ms 5.3922 ms 5.4032 ms]
Benchmarking walk_iterator/arbitrary_order_par_iter_with_rayon/65536: Collecting 100 samples in estimated 5.3222 s (walk_iterator/arbitrary_order_par_iter_with_rayon/65536
                        time:   [10.653 ms 10.668 ms 10.684 ms]
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe
walk_iterator/walk::<Dfs>/65536
                        time:   [5.5920 ms 5.6013 ms 5.6114 ms]
Found 11 outliers among 100 measurements (11.00%)
  11 (11.00%) high mild
walk_iterator/walk_par::<Dfs>/65536
                        time:   [5.4692 ms 5.4719 ms 5.4751 ms]
Found 12 outliers among 100 measurements (12.00%)
  2 (2.00%) high mild
  10 (10.00%) high severe
walk_iterator/walk::<Bfs>/65536
                        time:   [5.6978 ms 5.7045 ms 5.7117 ms]
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe
walk_iterator/walk_par::<Bfs>/65536
                        time:   [6.3015 ms 6.3094 ms 6.3177 ms]
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe
walk_iterator/walk::<PostOrder>/65536
                        time:   [5.7708 ms 5.7786 ms 5.7867 ms]
Benchmarking walk_iterator/walk_par::<PostOrder>/65536: Collecting 100 samples in estimated 5.2597 s (900 iterationswalk_iterator/walk_par::<PostOrder>/65536
                        time:   [5.8095 ms 5.8203 ms 5.8320 ms]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
Compared against main:
walk_iterator/arbitrary_order_iter/65536
                        time:   [5.2200 ms 5.2300 ms 5.2401 ms]
                        change: [-0.4891% -0.2789% -0.0362%] (p = 0.01 < 0.05)
                        Change within noise threshold.
walk_iterator/arbitrary_order_par_iter/65536
                        time:   [5.3813 ms 5.3922 ms 5.4032 ms]
                        change: [-1.3823% -1.0997% -0.8050%] (p = 0.00 < 0.05)
                        Change within noise threshold.
walk_iterator/arbitrary_order_par_iter_with_rayon/65536
                        time:   [10.653 ms 10.668 ms 10.684 ms]
                        change: [+0.8496% +1.1606% +1.4332%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe
walk_iterator/walk::<Dfs>/65536
                        time:   [5.5921 ms 5.6013 ms 5.6113 ms]
                        change: [-0.3022% -0.0069% +0.2876%] (p = 0.97 > 0.05)
                        No change in performance detected.
Found 11 outliers among 100 measurements (11.00%)
  11 (11.00%) high mild
walk_iterator/walk_par::<Dfs>/65536
                        time:   [5.4692 ms 5.4719 ms 5.4751 ms]
                        change: [-1.3387% -1.1855% -1.0315%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  2 (2.00%) high mild
  10 (10.00%) high severe
walk_iterator/walk::<Bfs>/65536
                        time:   [5.6978 ms 5.7045 ms 5.7116 ms]
                        change: [+0.1846% +0.3230% +0.4640%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe
walk_iterator/walk_par::<Bfs>/65536
                        time:   [6.3016 ms 6.3094 ms 6.3179 ms]
                        change: [-0.2852% -0.1233% +0.0450%] (p = 0.14 > 0.05)
                        No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe
walk_iterator/walk::<PostOrder>/65536
                        time:   [5.7709 ms 5.7786 ms 5.7868 ms]
                        change: [-0.4501% -0.2431% -0.0349%] (p = 0.02 < 0.05)
                        Change within noise threshold.
walk_iterator/walk_par::<PostOrder>/65536
                        time:   [5.8095 ms 5.8203 ms 5.8322 ms]
                        change: [-0.4904% -0.1507% +0.1680%] (p = 0.38 > 0.05)
                        No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
Compared to 66363f4
walk_iterator/arbitrary_order_iter/65536
                        time:   [5.2201 ms 5.2300 ms 5.2401 ms]
                        change: [-0.1295% +0.1349% +0.3727%] (p = 0.29 > 0.05)
                        No change in performance detected.
walk_iterator/arbitrary_order_par_iter/65536
                        time:   [5.3813 ms 5.3922 ms 5.4033 ms]
                        change: [-1.4007% -1.1615% -0.9184%] (p = 0.00 < 0.05)
                        Change within noise threshold.
walk_iterator/arbitrary_order_par_iter_with_rayon/65536
                        time:   [10.653 ms 10.668 ms 10.684 ms]
                        change: [+0.5182% +0.7970% +1.0498%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe
walk_iterator/walk::<Dfs>/65536
                        time:   [5.5922 ms 5.6013 ms 5.6113 ms]
                        change: [-1.1735% -0.9840% -0.7997%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 11 outliers among 100 measurements (11.00%)
  11 (11.00%) high mild
walk_iterator/walk_par::<Dfs>/65536
                        time:   [5.4692 ms 5.4719 ms 5.4751 ms]
                        change: [-2.2372% -2.0859% -1.9394%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  2 (2.00%) high mild
  10 (10.00%) high severe
walk_iterator/walk::<Bfs>/65536
                        time:   [5.6978 ms 5.7045 ms 5.7116 ms]
                        change: [-0.5319% -0.3112% -0.0935%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe
walk_iterator/walk_par::<Bfs>/65536
                        time:   [6.3015 ms 6.3094 ms 6.3178 ms]
                        change: [-1.0891% -0.8345% -0.6155%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe
walk_iterator/walk::<PostOrder>/65536
                        time:   [5.7707 ms 5.7786 ms 5.7868 ms]
                        change: [-0.6208% -0.3849% -0.1379%] (p = 0.00 < 0.05)
                        Change within noise threshold.
walk_iterator/walk_par::<PostOrder>/65536
                        time:   [5.8094 ms 5.8203 ms 5.8323 ms]
                        change: [+1.4464% +1.6854% +1.8934%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

Rust 1.91.0

TL;DR: It is either noise, or there is only a slight improvement compared to master, but the API is a lot better with this.

@orxfun
Copy link
Owner Author

orxfun commented Nov 23, 2025

TL;DR: It is either noise, or there is only a slight improvement compared to master, but the API is a lot better with this.

Thanks a lot for the benchmark! I agree that it looks like a small difference which might be noise, and like the api much better.

There are two related notes if you are interested:

  • There were some nice improvements in orx-parallel about parallelization on non-linear data structures in release 3.4.0. We don't yet use it here but might be interesting. If you have real use cases that requires parallelization, it could help to improve this repo.
  • In orx-linked-list, we implement Index on the list to access the nodes. You may see the relevant documentation here. In this version we use list[&idx] but it will change as list[idx] with the Copy release. This corresponds to tree.node(idx) here. I am not sure if we should or should not implement Index for the tree. Would be great if you let me know what you think.

@orxfun orxfun merged commit d07fd21 into main Nov 23, 2025
8 checks passed
@orxfun orxfun deleted the make-NodeIdx-Send branch November 23, 2025 19:16
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.

Make NodeIdx Send (or create new struct that is)

3 participants