Skip to content

Use setdefault for internal cache to ensure identity caching - #2217

Merged
seberg merged 1 commit into
NVIDIA:mainfrom
seberg:ft-fixes-setdefault
Jun 15, 2026
Merged

Use setdefault for internal cache to ensure identity caching#2217
seberg merged 1 commit into
NVIDIA:mainfrom
seberg:ft-fixes-setdefault

Conversation

@seberg

Copy link
Copy Markdown
Contributor

Switch selected cache write sites to setdefault/get-based patterns and keep explicit comments where setdefault is intentionally not used for primitive value caches.
(I just thought having setdefault mentioned might be nice in case of cargo-culting.)

Towards gh-2194; pytest-run-parallel does have a specific test for the GraphMemoryResource change. I could add an identity test for ComputeCapability probably (I doubt it actually matters for it, the use of setdefault just feels like a good pattern to me).

@copy-pr-bot

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@sebergseberg self-assigned this Jun 15, 2026
@sebergseberg added the bug Something isn't working label Jun 15, 2026
@github-actionsgithub-actionsBot added the cuda.core Everything related to the cuda.core module label Jun 15, 2026
@sebergseberg added P0 High priority - Must do! and removed cuda.core Everything related to the cuda.core module labels Jun 15, 2026
@sebergseberg changed the title Use setdefault-oriented cache updates in core hot pathsUse setdefault for internal cache to ensure identity cachingJun 15, 2026
Switch selected cache write sites to setdefault/get-based patterns and
keep explicit comments where setdefault is intentionally not used for
primitive value caches.
Signed-off-by: Sebastian Berg <sebastianb@nvidia.com>
@seberg
sebergforce-pushed the ft-fixes-setdefault branch from e30c476 to 3497ccfCompareJune 15, 2026 10:21
@github-actionsgithub-actionsBot added the cuda.core Everything related to the cuda.core module label Jun 15, 2026
@sebergseberg added this to the cuda.core v1.1.0 milestone Jun 15, 2026
@sebergseberg added cuda.core Everything related to the cuda.core module and removed cuda.core Everything related to the cuda.core module labels Jun 15, 2026
@seberg

Copy link
Copy Markdown
ContributorAuthor

/ok to test 3497ccf

@github-actions

Copy link
Copy Markdown

@mdboommdboom 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.

LGTM.

Do we want to add any AGENTS.md content about this pattern being preferred over functools.cache or the non-setdefault caching pattern?

@seberg

Copy link
Copy Markdown
ContributorAuthor

Do we want to add any AGENTS.md content about this pattern being preferred over functools.cache or the non-setdefault caching pattern?

Not sure AGENTS.md has such code styles yet? But if we want to add that I can follow-up.
FWIW, in most cases @functools.cache is nicer, but if it matters to return an idempotent result strictly then it doesn't (at the moment).

However, @rwgk: When scanning for @cache I didn't modify cuda-pathfinder since almost all places were just strings, etc. where it is good.
The one odd-one-out that caught my eye is load_nvidia_dynamic_lib if loading twice from two threads or the function being strictly idempotent can matter, then we need to fix it (I can do that in a follow-up if it seems important to you.)

@seberg
seberg merged commit 3833b93 into NVIDIA:mainJun 15, 2026
202 of 206 checks passed
github-actionsBot pushed a commit that referenced this pull request Jun 16, 2026
Removed preview folders for the following PRs:
- PR #2150
- PR #2217
- PR #2221
- PR #2224
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't workingcuda.coreEverything related to the cuda.core moduleP0High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@seberg@mdboom