Caches resolve_matching_names on AssetBase for all finder methods

[hougantc-nvda]

Description

Articulation.find_joints, find_bodies, and related finder methods delegate to resolve_matching_names, which runs a regex double-loop on every call. Nsight profiling of the pick-place task shows this costs ~881μs per call (~1.83ms / 1.5% of step time) for work that always returns the same result, since joint and body names are fixed after construction.

Cache the results using per-instance @functools.cache closures created in AssetBase._init_resolve_matching_names_caches(). On cache hit the decorator returns the stored result directly; the public wrapper methods return shallow copies so callers cannot mutate the cached data. The closures are instance-bound, so their cache dicts are freed automatically when the asset is garbage-collected.

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

• New feature (non-breaking change which adds functionality)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Summary

This PR caches results of find_joints and find_bodies on both the PhysX and Newton Articulation implementations using per-instance @functools.cache closures created in a new _init_finder_caches() helper. The public methods return shallow copies of cached lists to prevent mutation of the cache. Supporting tests verify identity isolation and independence between repeated calls.

Confidence Score: 5/5

Safe to merge — the caching logic is correct, tests verify mutation isolation, and no P0/P1 issues were found.

All findings are P2 or lower. The per-instance closure pattern is a valid approach for caching results that depend on immutable instance state. Shallow copies on return correctly prevent cache corruption. Tests explicitly validate that repeated calls return independent list objects. Changelog entries and version bumps are consistent.

No files require special attention.

Vulnerabilities

No security concerns identified.

Important Files Changed

Filename	Overview
source/isaaclab_physx/isaaclab_physx/assets/articulation/articulation.py	Adds _init_finder_caches() to create per-instance @cache closures; find_bodies and find_joints delegate to them and return shallow copies. Correct design given immutable body/joint names post-construction.
source/isaaclab_newton/isaaclab_newton/assets/articulation/articulation.py	Mirrors the PhysX changes: same _init_finder_caches() helper and updated find_bodies/find_joints wrappers. Symmetrical and consistent implementation.
source/isaaclab/test/assets/test_articulation_iface.py	Adds test_find_* tests covering return types, single-match, all-match, and mutation isolation (verifying returned lists are independent copies). Tests call _init_finder_caches() directly in the mock setup path.
source/isaaclab_physx/docs/CHANGELOG.rst	New 0.5.14 version heading added; version bumped to match extension.toml. RST formatting and Sphinx cross-references look correct.
source/isaaclab_newton/docs/CHANGELOG.rst	New 0.5.11 version heading added; version bumped to match extension.toml. RST formatting and Sphinx cross-references look correct.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Articulation
    participant Cache as @cache closure
    participant Resolver as resolve_matching_names

    Note over Articulation: __init__() calls _init_finder_caches()
    Articulation->>Cache: create _find_bodies_cached (per-instance)
    Articulation->>Cache: create _find_joints_cached (per-instance)

    Caller->>Articulation: find_bodies(name_keys)
    Articulation->>Articulation: normalize keys to tuple
    Articulation->>Cache: _find_bodies_cached(keys, preserve_order)
    alt Cache miss (first call)
        Cache->>Resolver: resolve_matching_names(keys, self.body_names, preserve_order)
        Resolver-->>Cache: (list[int], list[str])
        Cache-->>Articulation: cached result
    else Cache hit
        Cache-->>Articulation: stored result (no resolver call)
    end
    Articulation->>Articulation: list(result[0]), list(result[1]) shallow copy
    Articulation-->>Caller: (list[int], list[str]) independent copy

Loading

_{Reviews (2): Last reviewed commit: "Use functools.cache for find_joints and ..." | Re-trigger Greptile}

[isaaclab-review-bot]

PR Review: Caches find_joints and find_bodies on Articulation

Nice optimization — the Nsight profiling data makes a clear case for this, and the implementation is clean. Caching deterministic regex lookups that always return the same result is a solid win for hot-loop performance.

What looks good:

• Shallow-copy defense (list(result[0]), list(result[1])) correctly prevents callers from mutating cached data
• Tests explicitly verify identity independence (is not) and mutation isolation — exactly the right tests for this change
• Cache key construction properly normalizes both str and Sequence[str] inputs
• Both PhysX and Newton implementations are kept in sync

Minor observations (non-blocking):

[hougantc-nvda]

@greptileai

[greptile-apps]

Tip:

Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

[AntoineRichard]

Thanks for the optimization, I think it's important some of these results get cached to avoid overhead. However, right now, this PR misses the rest of the assets (RigidObjectCollection, RigidObject) as well as some other properties in the Articulation (Tendons, SpatialTendons). Could you extend the pattern to these. Also I would consider relying on an LRU cache rather than our own custom caching. I'm not completely against the custom caching, but I'd be curious to know why a custom cache was favored over the LRU cache.