Skip to content

[rocjitsu] Add intrusive shared ownership utilities and migrate consumers - #14539

Merged
IanWood1 merged 6 commits into
developfrom
users/IanWood1/intrusive-shared-ptr-20261027
Nov 3, 2026
Merged

IanWood1 merged 6 commits into
developfrom
users/IanWood1/intrusive-shared-ptr-20261027

Conversation

@IanWood1

@IanWood1 IanWood1 commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Motivation

Give shared VM snapshots and page-policy epochs explicit control over reference-count placement to avoid false sharing. Also support shared ownership without atomic counting when ownership stays within one analysis invocation.

Technical Details

Add IntrusiveSharedPtr<T> with strong ownership and no weak ownership. Object-provided add_ref() and release_ref() hooks determine counting and thread safety. Provide three storage helpers in the same header:

  • RefCounted<T> for compact storage with atomic reference counting.
  • RefCountedCacheAligned<T> to isolate the value from its atomic counter.
  • RefCountedNonAtomic<T> for ownership confined to one thread or externally synchronized across all handles sharing the object.

Use the aligned helper for shared GpuVmAccess snapshots and the page-table mutation epoch, replacing their ad hoc alignment. Preserve ordinary snapshot value copies and epoch invalidation ordering.

Use the non-atomic helper for RestoredSgprFacts in indirect-branch analysis. Its backing vector is shared only among local flow states within one synchronous analysis invocation. Preserve copy-on-write detachment before mutation, including when sibling branches share restored register facts.

Issue Tracking

Related to #10852.

@therock-pr-bot

therock-pr-bot Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@IanWood1
IanWood1 force-pushed the users/IanWood1/intrusive-shared-ptr-20261027 branch from 11543fb to 3f34d61 Compare October 29, 2026 06:31
@IanWood1 IanWood1 changed the title [rocjitsu] Add intrusive shared pointer and storage helpers [rocjitsu] Add intrusive shared ownership for VM snapshots and epochs Oct 29, 2026
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@IanWood1
IanWood1 marked this pull request as ready for review October 29, 2026 18:11
@IanWood1
IanWood1 requested review from a team and atgutier as code owners October 29, 2026 18:11
Copilot AI balanced review requested due to automatic review settings October 29, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@IanWood1 IanWood1 changed the title [rocjitsu] Add intrusive shared ownership for VM snapshots and epochs [rocjitsu] Add intrusive shared ownership utilities and migrate consumers Oct 29, 2026

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a few inline comments

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One comment, plus 2 inline replies

Comment thread emulation/rocjitsu/lib/util/README.md Resolved
Keep ownership hooks private and provide consistent read-only count queries
across storage helpers. Share the VM mutation-epoch alias across producers,
consumers, and tests, and clarify helper documentation and test naming.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Reject invalid forwarding arguments and wrapper self-copy construction so
deleted special members remain meaningful. Keep counter placement access
inside tests and isolate the shared epoch type from the VM registry.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@IanWood1
IanWood1 requested a review from kuhar November 3, 2026 07:04

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Compare shared handles directly in restored SGPR analysis, including const
handles, with tests distinguishing ownership identity from equal values.
Keep implementation friends private and epoch rationale with its documentation.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@IanWood1
IanWood1 merged commit 485fddd into develop Nov 3, 2026
69 checks passed
@IanWood1
IanWood1 deleted the users/IanWood1/intrusive-shared-ptr-20261027 branch November 3, 2026 19:13
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

rocjitsu post-commit fuzz passed

The 32-worker multiplexer campaigns for commit 485fddd463cb completed without recording a crash or hang.

  • ASan+UBSan (6-minute): 593,295 input executions (1,682.10 inputs/s)
  • TSan (6-minute): 537,464 input executions (1,536.15 inputs/s)

View the workflow run.

@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants