Skip to content

perf: use Box<Node> instead of Rc<Node> in BTreeMap iterator#360

Merged
maksymar merged 2 commits intomainfrom
maksym/box
Jul 7, 2025
Merged

perf: use Box<Node> instead of Rc<Node> in BTreeMap iterator#360
maksymar merged 2 commits intomainfrom
maksym/box

Conversation

@maksymar
Copy link
Copy Markdown
Contributor

@maksymar maksymar commented Jul 7, 2025 •
edited
Loading

This PR replaces the use of reference-counted Rc<Node> pointers with unique Box<Node> allocations in the BTreeMap iterator to reduce overhead.

btreemap

  instructions:
    status:   Improvements detected 🟢
    counts:   [total 285 | regressed 0 | improved 9 | new 0 | unchanged 276]
    change:   [max 0 | p75 -97.33K | median -147.22K | p25 -481.26K | min -5.34M]
    change %: [max 0.00% | p75 -0.02% | median -0.03% | p25 -0.06% | min -3.25%]

btreeset

  instructions:
    status:   Improvements detected 🟢
    counts:   [total 100 | regressed 0 | improved 2 | new 0 | unchanged 98]
    change:   [max +625.36K | p75 -288 | median -27.44K | p25 -80.94K | min -377.91K]
    change %: [max +0.05% | p75 -0.07% | median -0.30% | p25 -1.17% | min -2.35%]

Previous change #356 was adding Rc<Node> (vs simple moving) which added some improvements as well as regressions.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 7, 2025

canbench 🏋 (dir: ./benchmarks/memory_manager) 7b20e2d 2025-07-07 09:32:16 UTC

✅ ./benchmarks/memory_manager/canbench_results.yml is up to date
📦 canbench_results_memory-manager.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 3 | regressed 0 | improved 0 | new 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 3 | regressed 0 | improved 0 | new 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 3 | regressed 0 | improved 0 | new 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 7, 2025

canbench 🏋 (dir: ./benchmarks/btreemap) 7b20e2d 2025-07-07 09:34:07 UTC

✅ ./benchmarks/btreemap/canbench_results.yml is up to date
📦 canbench_results_btreemap.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   Improvements detected 🟢
    counts:   [total 285 | regressed 0 | improved 9 | new 0 | unchanged 276]
    change:   [max 0 | p75 -97.33K | median -147.22K | p25 -481.26K | min -5.34M]
    change %: [max 0.00% | p75 -0.02% | median -0.03% | p25 -0.06% | min -3.25%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 285 | regressed 0 | improved 0 | new 0 | unchanged 285]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 285 | regressed 0 | improved 0 | new 0 | unchanged 285]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------

Only significant changes:
| status | name                               | calls |     ins |  ins Δ% | HI |  HI Δ% | SMI |  SMI Δ% |
|--------|------------------------------------|-------|---------|---------|----|--------|-----|---------|
|   -    | btreemap_v2_scan_values_rev_1k_0b  |       |   1.21M |  -2.21% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_scan_iter_rev_1k_0b    |       |   1.23M |  -2.33% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_scan_iter_1k_0b        |       |   1.23M |  -2.51% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_scan_values_1k_0b      |       |   1.21M |  -2.56% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_range_count_1k_10kib   |       |   2.40M |  -2.69% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_scan_keys_20_10mib     |       |  18.47M |  -3.25% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_scan_keys_rev_20_10mib |       |  18.47M |  -3.25% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_range_count_20_10mib   |       |  18.47M |  -3.25% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreemap_v2_contains_10mib_values  |       | 142.21M |  -3.25% |  0 |  0.00% |   0 |   0.00% |

ins = instructions, HI = heap_increase, SMI = stable_memory_increase, Δ% = percent change

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 7, 2025

canbench 🏋 (dir: ./benchmarks/compare) 7b20e2d 2025-07-07 09:33:22 UTC

✅ ./benchmarks/compare/canbench_results.yml is up to date
📦 canbench_results_compare.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 18 | regressed 0 | improved 0 | new 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 18 | regressed 0 | improved 0 | new 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 18 | regressed 0 | improved 0 | new 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 7, 2025

canbench 🏋 (dir: ./benchmarks/btreeset) 7b20e2d 2025-07-07 09:32:34 UTC

✅ ./benchmarks/btreeset/canbench_results.yml is up to date
📦 canbench_results_btreeset.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   Improvements detected 🟢
    counts:   [total 100 | regressed 0 | improved 2 | new 0 | unchanged 98]
    change:   [max +625.36K | p75 -288 | median -27.44K | p25 -80.94K | min -377.91K]
    change %: [max +0.05% | p75 -0.07% | median -0.30% | p25 -1.17% | min -2.35%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 100 | regressed 0 | improved 0 | new 0 | unchanged 100]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 100 | regressed 0 | improved 0 | new 0 | unchanged 100]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------

Only significant changes:
| status | name               | calls |   ins |  ins Δ% | HI |  HI Δ% | SMI |  SMI Δ% |
|--------|--------------------|-------|-------|---------|----|--------|-----|---------|
|   -    | btreeset_range_u32 |       | 7.38M |  -2.28% |  0 |  0.00% |   0 |   0.00% |
|   -    | btreeset_range_u64 |       | 7.42M |  -2.35% |  0 |  0.00% |   0 |   0.00% |

ins = instructions, HI = heap_increase, SMI = stable_memory_increase, Δ% = percent change

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 7, 2025

canbench 🏋 (dir: ./benchmarks/vec) 7b20e2d 2025-07-07 09:32:22 UTC

✅ ./benchmarks/vec/canbench_results.yml is up to date
📦 canbench_results_vec.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@maksymar maksymar requested a review from Copilot July 7, 2025 09:38
Copilot AI reviewed Jul 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the use of reference-counted Rc<Node> pointers with unique Box<Node> allocations in the BTreeMap iterator to reduce overhead, and it updates recorded instruction counts in benchmark result files.

  • Swap out Rc<Node> for Box<Node> in Cursor and iteration code
  • Remove the now-unused std::rc::Rc import
  • Refresh instructions values in canbench_results.yml for both BTreeSet and BTreeMap benchmarks

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/btreemap/iter.rs Replaced Rc::new(node) with Box::new(node) and dropped Rc
benchmarks/btreeset/canbench_results.yml Updated measured instruction counts for BTreeSet benchmarks
benchmarks/btreemap/canbench_results.yml Updated measured instruction counts for BTreeMap benchmarks

@maksymar maksymar marked this pull request as ready for review July 7, 2025 09:41
@maksymar maksymar requested a review from a team as a code owner July 7, 2025 09:41
@maksymar maksymar changed the title perf: use Box<Node> instead of Rc<Node> perf: use Box<Node> instead of Rc<Node> in BTreeMap iterator Jul 7, 2025
@maksymar maksymar merged commit 4606637 into main Jul 7, 2025
20 checks passed
@maksymar maksymar deleted the maksym/box branch July 7, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@michael-weigelt michael-weigelt michael-weigelt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maksymar @michael-weigelt