Repository navigation
perf(manager): apply the render snapshot outside its lock - #4
Merged
Merged
Conversation
ApplyLatestRenderState moved every body and geom component while holding the snapshot lock, which the stepping thread needs to publish the next frame. On a 140-body, 485-geom robot that held the stepper behind the game thread for most of each frame: the simulation ran at about a quarter of real time with MuJoCo itself idle 95% of the time. Copy the snapshot out under the lock (microseconds) and apply the copy after releasing it. The buffer is game-thread only and reused between calls. The same model now steps at real time at a 1 ms timestep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The copied snapshot remains coherent while substantially reducing physics-thread lock contention.
0 open findings
What changed in this PR
Moves render-state application outside the snapshot lock, preventing physics-step stalls.
Changes:
- Copies the latest snapshot while locked.
- Applies transforms and records metadata after unlocking.
| File | Description |
|---|---|
Source/URLab/Private/MuJoCo/Core/AMjManager.cpp |
Shortens lock duration during render-state application. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
finger563
added a commit
to rammp-org/ramms-sim
that referenced
this pull request
Oct 10, 2026
- unreal-robotics-lab: rammp-org/UnrealRoboticsLab#4 is merged; point at the merge on ramms/v0.6.0-beta (bd11e5d). - RammsCore, RammsMujocoSupport, RammsPrivateAssets: the PR heads with the Copilot review fixes (ramms-core#30, ramms-mujoco-support#7, ramms-private-assets#7). Re-point these at their merged commits before this merges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
finger563
added a commit
to rammp-org/ramms-sim
that referenced
this pull request
Oct 10, 2026
- RammsCore: 0609d35 (rammp-org/ramms-core#30, squash of c79ffe6) - RammsMujocoSupport: c82f4f4 (rammp-org/ramms-mujoco-support#7, squash of 3998ecf) - RammsPrivateAssets: 63e1128 (rammp-org/ramms-private-assets#7) - unreal-robotics-lab stays at bd11e5d (rammp-org/UnrealRoboticsLab#4) Each merged tree is identical to the PR head that was built and tested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
finger563
added a commit
to rammp-org/ramms-sim
that referenced
this pull request
Oct 10, 2026
* chore: bump plugin pointers for lift-drive curb negotiation - RammsCore: URammsCurbNegotiationController and the 4-bar leg kinematics, plus a physics clock and backend-run speed loops on the robot base (rammp-org/ramms-core#30) - RammsMujocoSupport: wheel speed loops at the physics rate, mjData time (rammp-org/ramms-mujoco-support#7) - unreal-robotics-lab: apply the render snapshot outside its lock, which takes the roller lift-drive from ~0.25x to real time (rammp-org/UnrealRoboticsLab#4) - RammsPrivateAssets: curb test levels for both lift-drives, the roller robot's meshes/camera/physics, symmetric linkage limits, wheel gains (rammp-org/ramms-private-assets#7) The pointers are the PR branch heads; re-point them at the merged commits before this merges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: re-point plugins after review fixes; URLab at its merged commit - unreal-robotics-lab: rammp-org/UnrealRoboticsLab#4 is merged; point at the merge on ramms/v0.6.0-beta (bd11e5d). - RammsCore, RammsMujocoSupport, RammsPrivateAssets: the PR heads with the Copilot review fixes (ramms-core#30, ramms-mujoco-support#7, ramms-private-assets#7). Re-point these at their merged commits before this merges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: re-point RammsCore at the second round of review fixes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: point the curb-negotiation plugins at their merged commits - RammsCore: 0609d35 (rammp-org/ramms-core#30, squash of c79ffe6) - RammsMujocoSupport: c82f4f4 (rammp-org/ramms-mujoco-support#7, squash of 3998ecf) - RammsPrivateAssets: 63e1128 (rammp-org/ramms-private-assets#7) - unreal-robotics-lab stays at bd11e5d (rammp-org/UnrealRoboticsLab#4) Each merged tree is identical to the PR head that was built and tested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AAMjManager::ApplyLatestRenderStateused to apply the render snapshot to every articulation and quick-convert component while holding the snapshot lock. The stepping thread needs that same lock to publish the next frame, so it waited for the game thread for most of every frame.With the lift-drive roller model (about 140 bodies and 485 geoms), the simulation ran at about 0.25× real time while MuJoCo itself sat idle about 95% of the time. Profiling showed
PushRenderStatewaiting about 7.4 ms per step.The fix copies the snapshot out under the lock, which takes microseconds, and applies the copy after releasing it. The copy buffer is used only on the game thread and is reused between calls, so it keeps its allocations.
LastAppliedRenderFrameIdandLastAppliedRenderSimTimeare still recorded from the frame that was applied.Result: the same model steps at 0.97–1.0× real time at a 1 ms timestep (about 485 physics iterations/s → about 1000).
Testing
RammsEditorbuild (UE 5.8) in ramms-sim.Scripts/pie_curb/run_curb.sh) pass for both chassis.🤖 Generated with Claude Code