Skip to content

refactor(noq): atomic path ref counts#626

Merged
Frando merged 6 commits into
mainfrom
Frando/path-refcount-atomic
May 6, 2026
Merged

refactor(noq): atomic path ref counts#626
Frando merged 6 commits into
mainfrom
Frando/path-refcount-atomic

Conversation

@Frando
Copy link
Copy Markdown
Member

@Frando Frando commented Apr 29, 2026

Description

Mirror the recent ConnectionRef/EndpointRef refactor (c1d7ed2) for path reference counts. Cloning a Path or WeakPathHandle and calling Path::weak_handle/WeakPathHandle::upgrade no longer lock the connection state to bump a counter. Instead they fetch_add an atomic.

The state mutex is still taken on initial construction and on the last drop, since PathRef entries are allocated lazily per PathId and we need to clear path_refs plus any cached final_path_stats when the count hits zero.

Breaking Changes

None.

Change checklist

  • Self-review.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/noq/pr/626/docs/noq/

Last updated: 2026-05-06T13:17:52Z

@Frando Frando force-pushed the Frando/path-refcount-atomic branch 2 times, most recently from 453cca8 to 19daa94 Compare April 29, 2026 18:38
@Frando Frando force-pushed the Frando/path-refcount-atomic branch from 19daa94 to abcc1c7 Compare April 29, 2026 18:38
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

Performance Comparison Report

697b5a8e04e5cf3d7a40d1792fa0e5646cb1a1e4 - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5979.2 Mbps 7965.0 Mbps -24.9% 98.1% / 99.3%
medium-concurrent 5455.2 Mbps 8070.0 Mbps -32.4% 96.6% / 98.0%
medium-single 4473.6 Mbps 4670.0 Mbps -4.2% 95.7% / 98.0%
small-concurrent 3856.4 Mbps 5176.7 Mbps -25.5% 97.7% / 99.7%
small-single 3586.6 Mbps 4755.0 Mbps -24.6% 96.5% / 98.8%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3060.3 Mbps 4093.4 Mbps -25.2%
lan 782.5 Mbps 810.4 Mbps -3.4%
lossy 69.8 Mbps 69.8 Mbps ~0%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 23.4% slower on average

---
abcc1c74cdd224791142a36559206a24da4a84f4 - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5905.4 Mbps 8042.9 Mbps -26.6% 99.8% / 152.0%
medium-concurrent 5416.3 Mbps 7668.3 Mbps -29.4% 92.8% / 97.8%
medium-single 4151.3 Mbps 4748.9 Mbps -12.6% 93.4% / 101.0%
small-concurrent 3808.0 Mbps 5326.8 Mbps -28.5% 96.4% / 103.0%
small-single 3580.8 Mbps 4891.9 Mbps -26.8% 99.3% / 153.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3212.4 Mbps 4079.5 Mbps -21.3%
lan 782.4 Mbps 810.4 Mbps -3.5%
lossy 69.9 Mbps 69.9 Mbps ~0%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 24.4% slower on average

---
ed1703f5b9afcbc610ee63316704a89e1c350c4f - artifacts

No results available

---
abcc1c74cdd224791142a36559206a24da4a84f4 - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5636.9 Mbps 7877.9 Mbps -28.4% 95.6% / 99.7%
medium-concurrent 5344.0 Mbps 7934.5 Mbps -32.6% 95.6% / 148.0%
medium-single 4229.4 Mbps 4749.3 Mbps -10.9% 98.6% / 151.0%
small-concurrent 3891.0 Mbps 5144.2 Mbps -24.4% 93.1% / 103.0%
small-single 3484.0 Mbps 4772.0 Mbps -27.0% 94.4% / 104.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3074.2 Mbps 4106.9 Mbps -25.1%
lan 782.4 Mbps 810.3 Mbps -3.4%
lossy 69.8 Mbps 55.9 Mbps +25.0%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 25.2% slower on average

---
01358579f722a9dc099ef7c30013769301135305 - artifacts

No results available

---
4a55e4f776dec8c12bab051845474e2e3fc2b77b - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5776.8 Mbps 7702.0 Mbps -25.0% 97.3% / 98.7%
medium-concurrent 5308.4 Mbps 7712.6 Mbps -31.2% 95.5% / 97.2%
medium-single 4469.0 Mbps 4749.0 Mbps -5.9% 95.9% / 97.9%
small-concurrent 3856.8 Mbps 5483.2 Mbps -29.7% 97.8% / 100.0%
small-single 3528.0 Mbps 4827.7 Mbps -26.9% 95.8% / 98.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3058.6 Mbps 4078.7 Mbps -25.0%
lan 782.4 Mbps 810.3 Mbps -3.4%
lossy 69.8 Mbps 69.9 Mbps ~0%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 24.2% slower on average

---
039c31af263bef44d83dc9c8b1241cfd9f153f87 - artifacts

No results available

---
431edff42bd20ac3bfd234d3243ce5af46cbf65e - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5479.4 Mbps 7883.2 Mbps -30.5% 96.7% / 147.0%
medium-concurrent 5378.0 Mbps 8032.7 Mbps -33.0% 97.7% / 149.0%
medium-single 4186.5 Mbps 4749.1 Mbps -11.8% 89.8% / 97.9%
small-concurrent 3866.5 Mbps 5466.3 Mbps -29.3% 95.7% / 102.0%
small-single 3556.3 Mbps 4802.4 Mbps -25.9% 94.9% / 102.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3086.9 Mbps 4096.6 Mbps -24.6%
lan 782.4 Mbps 810.3 Mbps -3.4%
lossy 69.8 Mbps 69.8 Mbps ~0%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 26.4% slower on average

@n0bot n0bot Bot added this to iroh Apr 29, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh Apr 29, 2026
@Frando Frando marked this pull request as ready for review April 30, 2026 07:40
@Frando Frando requested a review from flub May 5, 2026 14:11
Copy link
Copy Markdown
Collaborator

@flub flub left a comment

Choose a reason for hiding this comment

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

I think this is correct.

But I also wonder if PathRef could be split in two halves: PathRefOwner, stored in the state, and PathRef handed out to the users (Path, and WeakPathHandle). That way PathRef could automatically handle the reference count increment on clone and the number of places where you have to carefully think about manually manipulating the PathRef::ref_count would reduce.

@Frando Frando force-pushed the Frando/path-refcount-atomic branch from ed1703f to abcc1c7 Compare May 6, 2026 12:26
Copy link
Copy Markdown
Collaborator

@flub flub left a comment

Choose a reason for hiding this comment

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

Heh, I guess you liked the idea of splitting up PathRef. Still LGTM.

Comment thread noq/src/path.rs

impl Drop for Path {
fn drop(&mut self) {
self.path_ref.on_drop(&self.conn);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, I didn't forsee this. A bit sad, but still an improvement I guess.

@Frando Frando added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit c64cf98 May 6, 2026
37 of 38 checks passed
@Frando Frando deleted the Frando/path-refcount-atomic branch May 6, 2026 17:09
@github-project-automation github-project-automation Bot moved this from 🚑 Needs Triage to ✅ Done in iroh May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants