Dense startup time profiling

[AntoineRichard]

Description

Allows for dense startup time profiling.

Type of change

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

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 introduces a dense startup-time profiling benchmark (benchmark_startup.py) that wraps each phase of an IsaacLab environment's startup sequence (app launch, Python imports, task config resolution, env creation, and first step) in its own cProfile session, emitting per-function timings as SingleMeasurement entries through the standard benchmark backend. It also extends SummaryMetrics to dynamically render any benchmark phases it doesn't already handle by name, provides a default whitelist YAML config for dashboard-stable function tracking, and ships corresponding documentation and a minor version bump.

Key changes:

• New: scripts/benchmarks/benchmark_startup.py — five-phase cProfile benchmark script with optional whitelist-mode function filtering
• New: scripts/benchmarks/startup_whitelist.yaml — default per-phase fnmatch pattern whitelist
• Modified: scripts/benchmarks/utils.py — adds parse_cprofile_stats() helper using the internal pstats.Stats.stats dict
• Modified: source/isaaclab/isaaclab/test/benchmark/backends.py — SummaryMetrics now renders unknown phases dynamically instead of silently dropping them
• Docs / metadata: benchmarks.rst, CHANGELOG.rst, extension.toml updated accordingly

Three logic-level issues were found in benchmark_startup.py: (1) env_cfg.seed is unconditionally overridden with None when --seed is not supplied, silently clearing any config-level seed; (2) env_creation_profile is not disabled if gym.make() raises a BaseException (e.g. KeyboardInterrupt), because the handler uses except Exception instead of except BaseException or a finally; and (3) the first-step action sampling hard-codes a 1D continuous Box space assumption via .single_action_space.shape[0], which will raise or produce incorrect results for discrete or multi-dimensional action spaces.

Confidence Score: 2/5

• The new benchmark script has three logic-level bugs that could cause silent correctness issues or unexpected crashes during profiling runs.
• The core infrastructure changes (backends.py dynamic rendering, utils.py parse_cprofile_stats) are solid and well-guarded. However, benchmark_startup.py carries three reproducible logic bugs: unconditional seed override strips config-level seeds; bare except Exception leaves the profile enabled on keyboard interrupts or SystemExit; and the action-space shape assumption will crash on non-1D Box environments. These do not affect the existing library but will affect correctness and usability of the new benchmark tool.
• scripts/benchmarks/benchmark_startup.py requires attention for the seed override, BaseException handling, and action space shape assumption.

Important Files Changed

Filename	Overview
scripts/benchmarks/benchmark_startup.py	New startup profiling script with three logic issues: unconditional seed override with None, env_creation_profile not disabled on BaseException from gym.make(), and hard-coded 1D Box action space assumption for first_step sampling.
scripts/benchmarks/utils.py	Adds parse_cprofile_stats() helper. Relies on the internal pstats.Stats.stats dict (acknowledged in a code comment). Whitelist pattern-matching and placeholder emission logic looks correct. Minor: print_startup_summary defined in a prior thread is noted as dead code.
source/isaaclab/isaaclab/test/benchmark/backends.py	Adds a dynamic fallback renderer for unknown phases in SummaryMetrics._print_summary(). Implementation is correct and consistent with existing frametime rendering. known_phases set is defined inline; if a future developer adds another hard-coded phase after this loop, it could render twice — minor future-maintenance concern.
scripts/benchmarks/startup_whitelist.yaml	Default whitelist config covering app_launch, env_creation, and first_step phases. Pattern syntax is consistent with fnmatch; no issues found.
docs/source/testing/benchmarks.rst	Adds comprehensive documentation for the startup profiling benchmark and updates the SummaryMetrics description. Content is accurate relative to the implementation.

Sequence Diagram

sequenceDiagram
    participant CLI as __main__
    participant LS as launch_simulation()
    participant M as main()
    participant GYM as gym.make()
    participant ENV as env
    participant BP as BaseIsaacLabBenchmark
    participant BE as Backend (SummaryMetrics etc.)

    CLI->>LS: app_launch_profile.enable() → with launch_simulation(env_cfg, args_cli)
    LS-->>CLI: context entered (app_launch_profile.disable())
    CLI->>M: main(env_cfg, app_launch_profile, app_launch_wall_ms)
    M->>GYM: env_creation_profile.enable() → gym.make(task, cfg)
    GYM-->>M: env
    M->>ENV: env.reset()  [inner try/finally disables env_creation_profile]
    ENV-->>M: obs
    M->>ENV: env.step(actions)  [first_step_profile try/finally]
    ENV-->>M: obs, reward, ...
    M->>BP: benchmark.add_measurement(phase, SingleMeasurement) ×N phases
    M->>BP: benchmark.update_manual_recorders()
    M->>BP: benchmark._finalize_impl()
    BP->>BE: backend.finalize(output_path, output_filename)
    BE-->>CLI: JSON + console summary printed
    M->>ENV: env.close()  [outer finally]

Loading

_{Last reviewed commit: "Document reliance on..."}

[AntoineRichard]

@greptile

[AntoineRichard]

@greptile

[greptile-apps]

env_creation_profile left enabled on BaseException

except Exception does not catch KeyboardInterrupt, SystemExit, or any other BaseException subclass. If gym.make() is interrupted by a signal or a hard abort, the profile object is left in the enabled state for the remainder of the process lifetime, which silently continues accumulating CPU-time into the stale profile.

The standard pattern is to use a finally clause (or a bare except) so the profile is always stopped:

env_creation_profile.enable()
try:
    env = gym.make(args_cli.task, cfg=env_cfg)
except BaseException:
    env_creation_profile.disable()
    raise

Or, more idiomatically, fold the disable into a try/finally around the whole block, the same way first_step_profile is handled a few lines below.

[isaaclab-review-bot]

Startup Profiling Benchmark — Code Review

Overall this is a solid, well-structured addition. The five-phase profiling architecture is clean, the whitelist/fnmatch system is genuinely useful for dashboard stability, and the error handling has clearly gotten multiple passes of improvement. A few issues below — one actual bug, a couple of design nits.

Verdict: Approve with nits. The seed bug (comment 1) should be fixed before merge; the rest are minor.

[isaaclab-review-bot]

Bug: unconditional seed override clobbers config default.

env_cfg.seed = args_cli.seed

When --seed is not passed, args_cli.seed is None. This unconditionally overwrites whatever env_cfg.seed was — either the task config's default or the conditional assignment at module-level line 166.

Line 166 already does the right thing:

env_cfg.seed = args_cli.seed if args_cli.seed is not None else env_cfg.seed

This line should match that pattern, or just be removed since the module-level assignment already ran:

# Either:
env_cfg.seed = args_cli.seed if args_cli.seed is not None else env_cfg.seed
# Or just delete this line — line 166 already handled it.

[isaaclab-review-bot]

Nit: module-level seed assignment is redundant with main(). This line correctly does the conditional assignment, but then main() line 206 unconditionally overwrites it (see other comment). If you fix line 206 to be conditional too, consider removing one of the two assignments to avoid confusion about which one "wins".

Since env_cfg is passed into main() and main() also sets num_envs and device there, it'd be cleaner to do all config overrides in one place — inside main() — and remove this line.

[isaaclab-review-bot]

Minor: calling private _finalize_impl() directly.

I know other benchmark scripts (benchmark_non_rl.py) do this too, so this is a pre-existing pattern rather than a problem you introduced. Still worth noting: if BaseIsaacLabBenchmark ever gains a public finalize() wrapper (which it should), this should switch to it.

[isaaclab-review-bot]

Design note: cuda.is_initialized() check may miss lazy CUDA init.

At this point in the script, CUDA may not yet be initialized (torch was just imported but no tensor ops have run). If CUDA isn't initialized, is_initialized() returns False and the synchronize is skipped — which is correct, since there's nothing to sync. But it means the wall-clock imports_time_end doesn't account for any deferred GPU init that gets triggered later.

This is fine for a profiling script (you're measuring import time, not GPU init time), but worth a comment explaining the intent — especially since the same pattern at lines 225 and 245 does catch active GPU work.

[isaaclab-review-bot]

Observation (not a bug): external functions can appear with abbreviated labels. When an external function (e.g., torch.nn.modules.linear:forward) is directly called by an IsaacLab function, _make_label truncates the path to site-packages-relative or last-3-components. This is fine for readability but means the label isn't a valid Python import path — it's a display-only string. Worth a brief docstring note on _make_label that labels are for display, not programmatic use.

[isaaclab-review-bot]

Nit: single_action_space may not exist for all env types.

For DirectMARLEnv, the action space structure is different — single_action_space exists but might be a Dict space, not a Box, so .shape[0] would fail. The type signature of main() accepts DirectMARLEnvCfg, but this line assumes a flat action space.

Since this is a benchmarking script (not a general-purpose tool), this is acceptable if the docs/help text clarify it's for single-agent RL envs. But if MARL envs are intended to be supported, this needs a check.

[isaaclab-review-bot]

Good addition. The catch-all rendering for unknown phases is clean and doesn't break existing behavior. One minor style nit: the unit_str construction is duplicated between the frametime_phase block above and this new block. Could extract a helper like _format_measurement_value(measurement) to DRY it up, but that's cosmetic.

[ooctipus]

@AntoineRichard nice feature

def parse_cprofile_stats and cprofile related logic is added to startup time, it could also be beneficial to runtime dense breakdown do you plan to intrument these new feature to benchmark rsl_rl, or benchmark non_rl?