Use borrowed pointers in TYPE_CACHE instead of strong references#7384

youknowone

The method cache (TYPE_CACHE) was storing strong references (Arc clones) to cached attribute values, which inflated sys.getrefcount(). This caused intermittent test_memoryview failures where refcount assertions would fail depending on GC collection timing.

Store borrowed raw pointers instead. Safety is guaranteed because:

type_cache_clear() nullifies all entries during GC collection, before the collector breaks cycles
type_cache_clear_version() nullifies entries when a type is modified, before the source dict entry is removed
Readers use try_to_owned_from_ptr (safe_inc) to atomically validate and increment the refcount on cache hit

Summary by CodeRabbit

Refactor
- Optimized type cache memory management in the virtual machine for improved efficiency.

The method cache (TYPE_CACHE) was storing strong references (Arc clones) to cached attribute values, which inflated sys.getrefcount(). This caused intermittent test_memoryview failures where refcount assertions would fail depending on GC collection timing. Store borrowed raw pointers instead. Safety is guaranteed because: - type_cache_clear() nullifies all entries during GC collection, before the collector breaks cycles - type_cache_clear_version() nullifies entries when a type is modified, before the source dict entry is removed - Readers use try_to_owned_from_ptr (safe_inc) to atomically validate and increment the refcount on cache hit

coderabbitai

📝 Walkthrough

Walkthrough

The PR converts the type cache from storing owned PyObject references to borrowed pointers, reducing reference counting overhead. The take_value method is replaced with clear_value for cache invalidation. Cache insertion and clearing operations are updated to work with borrowed pointers while maintaining correctness with GC behavior.

Changes

Cohort / File(s)	Summary
Type Cache Reference Semantics `crates/vm/src/builtins/type.rs`	Changed `TypeCacheEntry` value field from owned `PyObjectRef` to borrowed pointer; replaced `take_value()` with `clear_value()` for cache invalidation; updated cache clearing and insertion paths to work with borrowed pointers; adjusted `PyType.modified` to nullify pointers instead of releasing strong references; added safety notes on GC behavior and cycle collection interactions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Add method cache for MRO lookups #7313: Directly related—this PR modifies the MCACHE structures and cache management functions originally introduced in that PR, changing the ownership semantics from owned to borrowed references.

Poem

🐰 Hop, hop! A cache so clever,
From owned refs to borrowed—light as a feather!
No counters to count, no drops to fear,
Just borrowed whispers in memory's ear. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately summarizes the main change: replacing strong references with borrowed pointers in the TYPE_CACHE, which is the core motivation and implementation of this pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/vm/src/builtins/type.rs`:
- Around line 84-89: Update the unsafe lifetime doc for the type cache to cover
both roots: state that entries referencing heap types are cleared by GC via
type_cache_clear() and type_cache_clear_version(), while static
(untracked/immortal) types are not collected and remain valid for the program
lifetime (refer to the static-type untracking logic). Mention both behaviors
explicitly so callers understand that cached pointers may be nulled during GC
for heap types but are permanently valid for static types.
- Around line 805-809: The type dict mutation pathway must always call the
cache-invalidation path before changing attributes; create and use a helper like
mutate_type_dict(&self, f: impl FnOnce(&mut PyAttributes)) that calls
self.modified() then invokes f on self.attributes.write(), and replace all
direct mutations (e.g., calls to self.attributes.write().insert, swap_remove,
remove) in the annotation/cache setters and the sites around the symbols
mentioned (lines handling __annotations_cache__, __annotate_func__, and the
blocks currently using insert/swap_remove) to use this helper so every
post-construction write funnels through the invalidation path.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 35c55559-b67c-4e53-ad9e-c8a48f63fa18

📥 Commits

Reviewing files that changed from the base of the PR and between 46abff6 and b7b23e7.

📒 Files selected for processing (1)

crates/vm/src/builtins/type.rs

…tPython#7384) The method cache (TYPE_CACHE) was storing strong references (Arc clones) to cached attribute values, which inflated sys.getrefcount(). This caused intermittent test_memoryview failures where refcount assertions would fail depending on GC collection timing. Store borrowed raw pointers instead. Safety is guaranteed because: - type_cache_clear() nullifies all entries during GC collection, before the collector breaks cycles - type_cache_clear_version() nullifies entries when a type is modified, before the source dict entry is removed - Readers use try_to_owned_from_ptr (safe_inc) to atomically validate and increment the refcount on cache hit

youknowone marked this pull request as ready for review March 8, 2026 12:58

coderabbitai Bot reviewed Mar 8, 2026

View reviewed changes

youknowone deleted the type-cache branch March 8, 2026 13:55

coderabbitai Bot mentioned this pull request Mar 27, 2026

Add global type mutex for thread-safe type operations #7416

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use borrowed pointers in TYPE_CACHE instead of strong references#7384