perf: Disable USD notice handlers during usd_replicate for 25-35% faster startup

[aloysbaillet]

Description

Summary

Disable IFabricUsd and SimulationManager USD notice handlers during bulk environment cloning in usd_replicate(), reducing startup time by 25-35% for manager-based tasks.

Problem

When usd_replicate() calls Sdf.CopySpec to clone thousands of environment prims, every single copy triggers IFabricUsd UsdNoticeListener::Handle(UsdNotice::ObjectsChanged) — an expensive Fabric↔USD sync operation. For Reach-Franka (4096 envs), this added 17 seconds (438 notice handler calls) during scene construction.

IsaacSim's own Cloner.disable_change_listener() already handles this by disabling the notice handler before cloning, but IsaacLab v3's replacement usd_replicate() function didn't replicate this behavior.

Additionally, the import path from isaacsim.core.simulation_manager import SimulationManager resolves to PhysxManager under IsaacLab's lazy_export module system, which lacks the enable_fabric_usd_notice_handler method. The correct import is from isaacsim.core.simulation_manager.impl.simulation_manager import SimulationManager.

Changes

source/isaaclab/isaaclab/cloner/cloner_utils.py:

• Add _disable_usd_notice_handlers() / _enable_usd_notice_handlers() helpers that mirror isaacsim.core.cloner.Cloner.disable_change_listener()
• Wrap usd_replicate() body in try/finally to disable handlers during bulk USD authoring
• Use correct full import path for SimulationManager

source/isaaclab/isaaclab/utils/string.py:

• Cache string_to_callable() with @functools.cache to avoid repeated importlib.import_module calls
• Fix triple ast.parse() call in is_lambda_expression()

Benchmark Results

Tested on DGXC (2x NVIDIA L40, 4096 envs, 1 GPU, warm run):

Task	Before	After	Improvement
Isaac-Reach-Franka-v0	29.0s	20.0s	-31%
Isaac-Velocity-Rough-H1-v0	37.9s	24.7s	-35%
Isaac-Velocity-Flat-Anymal-D-v0	29.8s	22.1s	-26%
Isaac-Cartpole-Direct-v0	8.9s	9.1s	~0% (minimal cloning)
Isaac-Ant-Direct-v0	11.9s	12.0s	~0% (minimal cloning)
No FPS regressions. All tasks pass.

Root Cause Analysis

Profiling with Nsight Systems showed IFabricUsd UsdNoticeListener::Handle consuming:

• 17.0s for Reach-Franka (438 calls)
• 9.9s for H1-Rough
• 22.9s for Dexsuite-Kuka

These notice handler calls happen during usd_replicate when Sdf.CopySpec modifies the USD stage. By disabling the handlers (as IsaacSim's own Cloner does), the Fabric sync is deferred to a single batch at the end.

Type of change

• Bug fix (non-breaking change which fixes an issue)

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 delivers a meaningful startup-time optimization (25–35% reduction for large environments) by mirroring what IsaacSim's own Cloner.disable_change_listener() does: suppressing IFabricUsd and SimulationManager USD notice handlers during bulk Sdf.CopySpec calls in usd_replicate(), so that Fabric↔USD synchronization is deferred to a single batch instead of firing on every spec copy. A secondary optimization caches string_to_callable() results and eliminates a triple ast.parse() call in is_lambda_expression().

Key changes:

• cloner_utils.py: New _disable_usd_notice_handlers / _enable_usd_notice_handlers helpers wrap usd_replicate() in a try/finally; all per-depth Sdf.ChangeBlock calls are consolidated into one.
• string.py: @functools.cache added to string_to_callable(); is_lambda_expression() fixed to parse the AST only once.

Issues found:

• The USD notice handler (enable_usd_notice_handler) is always restored to True in the finally block without first saving its original state. The fabric handler correctly saves and conditionally restores its state, but the USD handler does not — this asymmetry means a caller that had previously disabled the handler will have it silently re-enabled after usd_replicate() returns.
• UsdUtils.StageCache.Get().Insert(stage) permanently registers the stage in the global cache if it was not already there, with no corresponding cleanup. In normal Isaac Sim operation the stage is pre-registered, but in unit tests or lightweight scenarios this can prevent the stage from being garbage-collected.
• The consolidation of per-depth Sdf.ChangeBlock calls into a single outer block is a behavioral change from the original design (which committed after each depth to "stabilize composition"); this is likely safe for the tested assets but deserves an explanatory comment.
• @functools.cache on string_to_callable is unbounded and never cleared, which could accumulate memory in long-running training processes.

Confidence Score: 3/5

• The optimization is architecturally sound and well-benchmarked, but two correctness issues in handler state management should be resolved before merging.
• The core approach (mirroring IsaacSim's own Cloner pattern) is correct and the benchmarks show no regressions. However, the unconditional enable_usd_notice_handler(True) in the finally block without saving prior state is an asymmetric bug that could cause subtle issues if usd_replicate() is called within an outer disable context. The stage cache insertion side effect is a second correctness concern. These two P1 issues prevent a higher score.
• Pay close attention to source/isaaclab/isaaclab/cloner/cloner_utils.py, specifically the _disable_usd_notice_handlers / _enable_usd_notice_handlers pair and the stage cache insertion logic.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/cloner/cloner_utils.py	Adds _disable_usd_notice_handlers/_enable_usd_notice_handlers helpers and wraps usd_replicate() in try/finally to suppress Fabric sync overhead during bulk cloning. Two correctness concerns: (1) the USD notice handler state is not saved before disabling and is always unconditionally re-enabled (unlike the fabric handler which correctly saves/restores state), and (2) stage insertion into UsdUtils.StageCache without tracking prior membership risks a permanent cache side-effect. The ChangeBlock consolidation from per-depth to a single block is also a behavioral change that warrants a comment explaining why it is safe.
source/isaaclab/isaaclab/utils/string.py	Two clean optimizations: fixes triple ast.parse() redundancy in is_lambda_expression() by caching the tree, and adds @functools.cache to string_to_callable() to avoid repeated importlib.import_module costs. The cache is unbounded and process-lifetime, which may cause memory growth in long-running sessions — lru_cache(maxsize=N) would be safer.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant usd_replicate
    participant _disable_usd_notice_handlers
    participant SimulationManager
    participant UsdUtils_StageCache
    participant Sdf

    Caller->>usd_replicate: usd_replicate(stage, sources, destinations, ...)
    usd_replicate->>_disable_usd_notice_handlers: _disable_usd_notice_handlers(stage)
    _disable_usd_notice_handlers->>UsdUtils_StageCache: Insert(stage) → stage_id
    _disable_usd_notice_handlers->>SimulationManager: is_fabric_usd_notice_handler_enabled(stage_id)
    SimulationManager-->>_disable_usd_notice_handlers: fabric_was_enabled
    _disable_usd_notice_handlers->>SimulationManager: enable_fabric_usd_notice_handler(stage_id, False)
    _disable_usd_notice_handlers->>SimulationManager: enable_usd_notice_handler(False)
    _disable_usd_notice_handlers-->>usd_replicate: state dict

    rect rgb(200, 230, 200)
        Note over usd_replicate,Sdf: try block — bulk USD authoring with handlers suppressed
        usd_replicate->>Sdf: ChangeBlock (single block wrapping all depths)
        loop for each depth × source × env_id
            usd_replicate->>Sdf: CreatePrimInLayer(rl, dp)
            usd_replicate->>Sdf: CopySpec(src → dp)
            usd_replicate->>Sdf: author xformOp:translate / xformOp:orient
        end
        Sdf-->>usd_replicate: ChangeBlock closes → single batch notify
    end

    rect rgb(255, 220, 220)
        Note over usd_replicate,SimulationManager: finally block — always restore handlers
        usd_replicate->>SimulationManager: enable_fabric_usd_notice_handler(stage_id, True) [if was_enabled]
        usd_replicate->>SimulationManager: enable_usd_notice_handler(True) [unconditional ⚠️]
    end

    usd_replicate-->>Caller: return

Loading

_{Last reviewed commit: "perf: disable USD no..."}

[greptile-apps]

USD notice handler state not saved before disabling

SimulationManager.enable_usd_notice_handler(False) is called unconditionally without first checking whether it was already enabled. In _enable_usd_notice_handlers, it is then always restored to True — regardless of what its state was before usd_replicate() was called.

Contrast this with the fabric notice handler, where the initial state is checked with is_fabric_usd_notice_handler_enabled() and only restored if it was enabled. If any outer code (e.g. another nested cloning operation or a user-managed disable context) had already set the USD notice handler to False, calling usd_replicate() would silently re-enable it in the finally block — a side effect the caller would not expect.

The fix is to save and conditionally restore the USD notice handler state, mirroring the fabric handler pattern:

    try:
        from isaacsim.core.simulation_manager.impl.simulation_manager import SimulationManager

        state["fabric_notice_was_enabled"] = SimulationManager.is_fabric_usd_notice_handler_enabled(stage_id)
        if state["fabric_notice_was_enabled"]:
            SimulationManager.enable_fabric_usd_notice_handler(stage_id, False)
        # Save and conditionally disable the USD notice handler as well
        state["usd_notice_was_enabled"] = SimulationManager.is_usd_notice_handler_enabled()
        if state["usd_notice_was_enabled"]:
            SimulationManager.enable_usd_notice_handler(False)
        state["sim_manager_available"] = True

And in _enable_usd_notice_handlers:

        if state.get("usd_notice_was_enabled", False):
            SimulationManager.enable_usd_notice_handler(True)

(This assumes SimulationManager.is_usd_notice_handler_enabled() exists; if not, at minimum the existing asymmetry relative to the fabric handler should be documented.)

[greptile-apps]

Stage inserted into global cache without cleanup

UsdUtils.StageCache.Get().Insert(stage) inserts the stage into the global, process-wide USD stage cache. If the stage was not already registered in the cache, this call permanently adds it — and there is no corresponding removal, either in _disable_usd_notice_handlers or in the finally block of usd_replicate().

In Isaac Sim's normal runtime the active stage is typically already cached (Isaac Sim registers it at startup), so this is usually a no-op insertion. However, in unit tests or lightweight scenarios where the stage is created independently, calling usd_replicate() would silently inject it into the global cache and it would remain there for the lifetime of the process, preventing the stage from being garbage-collected.

Consider checking whether the stage was already in the cache and only storing the ID without re-insertion, or — if an insertion was needed — removing it in the finally path:

cache = UsdUtils.StageCache.Get()
cache_id = cache.GetId(stage)
if not cache_id.IsValid():
    cache_id = cache.Insert(stage)
    state["stage_inserted_by_us"] = True
else:
    state["stage_inserted_by_us"] = False
state["stage_id"] = cache_id.ToLongInt()
state["cache_id_obj"] = cache_id

Then in _enable_usd_notice_handlers, conditionally erase:

if state.get("stage_inserted_by_us", False):
    UsdUtils.StageCache.Get().Erase(state["cache_id_obj"])

[greptile-apps]

Redundant exception type in except clause

ImportError is a subclass of Exception, so except (ImportError, Exception) is equivalent to except Exception. Listing ImportError explicitly adds no additional behavior and may give the false impression that it is handled differently from other exceptions.

Suggested change

	except (ImportError, Exception) as e:
	except Exception as e:

[greptile-apps]

Redundant exception type in except clause

Same issue as in _disable_usd_notice_handlers — ImportError is already covered by except Exception.

Suggested change

	except (ImportError, Exception) as e:
	except Exception as e:

[greptile-apps]

Single Sdf.ChangeBlock changes prior per-depth composition semantics

The original code wrapped each depth level in its own Sdf.ChangeBlock(), which caused USD to flush pending change notices and re-compose the stage at the end of each depth. The accompanying comment explicitly noted this was done "to stabilize composition" for nested or interdependent specs.

The new code wraps all depths in one Sdf.ChangeBlock, deferring all notifications (and USD composition) until every Sdf.CopySpec across every depth has completed. For pure SDF layer writes this should be safe because Sdf.CopySpec operates on spec data rather than the composed stage. However, if any path deeper in the hierarchy is defined via composition arcs that reference an ancestor spec authored at a shallower depth in the same call, the composed view of those arcs won't be available until after the outer ChangeBlock closes.

This is likely fine for the tested robot/environment assets, but it is a semantic regression from the original design. If this change is intentional (as an additional performance improvement complementing the notice-handler suppression), please add a comment explaining why per-depth flushing is no longer needed, so future maintainers don't revert it without understanding the trade-off.

[greptile-apps]

@functools.cache is unbounded and never cleared

functools.cache is an alias for lru_cache(maxsize=None), which means every unique string ever passed to string_to_callable is retained for the process lifetime along with a strong reference to the resolved callable (and, transitively, its module). In long-running Isaac Sim training sessions with many distinct callable strings this cache will grow without bound and prevent modules from being garbage-collected.

Two additional edge-cases to be aware of:

1. Module reloading: if a downstream user calls importlib.reload(some_module), cached entries will silently continue returning the pre-reload callable.
2. Lambda expressions: eval(name) creates a new function object on each call, but with caching only one instance is ever created. This is harmless for pure lambdas, but could be surprising if someone expected independent closure objects.

Consider using @functools.lru_cache(maxsize=512) to cap memory usage, or document the unbounded nature clearly so maintainers know to call string_to_callable.cache_clear() if needed.