Skip to content

Avoid redundant TF lookups in getShapeTransformCache - #3803

Open
hugogo1998 wants to merge 3 commits into
moveit:mainfrom
hugogo1998:perfopt/moveit2-768
Open

Avoid redundant TF lookups in getShapeTransformCache#3803
hugogo1998 wants to merge 3 commits into
moveit:mainfrom
hugogo1998:perfopt/moveit2-768

Conversation

@hugogo1998

@hugogo1998 hugogo1998 commented Jul 24, 2026

Copy link
Copy Markdown

Description

getShapeTransformCache() currently calls canTransform() before each lookupTransform(), causing an extra TF-tree traversal when the transform is already available.

This change performs a single direct lookup on the common path. If it is not immediately available, it retries with shape_transform_cache_lookup_wait_time_. A missing transform skips only that frame, so the rest of the cache is still populated.

Results

Real tf2::BufferCore, mean cache-build time:

Collision shapes Current This change Speedup
4 2.06 µs 1.69 µs 1.22x
8 6.58 µs 4.41 µs 1.49x
16 11.69 µs 7.26 µs 1.61x
32 32.04 µs 20.68 µs 1.55x

Testing

Added coverage for a transform that arrives during the configured wait and for continuing after one frame is unavailable.

Built moveit_ros_planning and ran planning_scene_monitor_test in moveit/moveit2:humble-ci: 4/4 gtests passed; colcon reported 3 tests, 0 errors, 0 failures, 0 skipped.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend tutorials / documentation — not applicable; no user-facing change
  • Document API changes in MIGRATION.md — not applicable; no API change
  • Create tests — added timeout and partial-cache coverage
  • Include a screenshot if changing a GUI — not applicable
  • While waiting for the PR to be reviewed, review another open PR

getShapeTransformCache rebuilds on every octomap/sensor update and
walks the TF tree twice per shape-bearing frame: canTransform first
(a check), then lookupTransform (the actual compose). lookupTransform
already composes the transform and throws tf2::TransformException if
it's unavailable, which the function's existing try/catch already
handles, so the canTransform check duplicates work lookupTransform
does anyway.

Drop it and call lookupTransform directly inside the existing
try/catch. Cache contents are unchanged: lookupTransform returns the
same transform whether or not canTransform was called first.

One real behavioral difference worth being explicit about:
canTransform(..., shape_transform_cache_lookup_wait_time_) also
waits up to that duration (a ROS parameter,
"<robot_description>_planning.shape_transform_cache_lookup_wait_time",
defaulting to 0.05s) for a not-yet-available transform. Dropping it
removes that wait. When a transform is available at target_time (the
normal case), behavior is identical. If one is momentarily late, the
current code waits up to the configured duration; this throws
immediately, and the try/catch returns false for that update (the
next sensor update tries again). The wait-time parameter itself
becomes a no-op, since this was its only reader.
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ce4d704-08ad-456f-8c0d-25cedbd54960

📥 Commits

Reviewing files that changed from the base of the PR and between 4a5021b and 12194e2.

📒 Files selected for processing (2)
  • moveit_ros/planning/planning_scene_monitor/src/planning_scene_monitor.cpp
  • moveit_ros/planning/planning_scene_monitor/test/planning_scene_monitor_test.cpp

Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Changes

The planning scene monitor now resolves shape transforms per frame. It first performs a direct BufferCore lookup, then a timed buffer lookup. Missing transforms are skipped without aborting cache construction. Tests cover delayed and missing transforms.

Shape transform cache

Layer / File(s) Summary
Per-frame transform lookup
moveit_ros/planning/planning_scene_monitor/src/planning_scene_monitor.cpp
getShapeTransformCache() uses direct and timed lookups for each frame. It adds link, attached-body, and collision-object entries only when transforms are available.
Transform cache test coverage
moveit_ros/planning/planning_scene_monitor/test/planning_scene_monitor_test.cpp
The test fixture exposes transform-cache helpers. Tests cover delayed transforms and missing transforms for one of several links.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 12194

The change removes the configured wait for temporarily unavailable transforms and changes failures from skipping one affected frame to stopping the remaining cache work for that update. This is mergeable with owner awareness because transient TF delays could leave less of the cache populated during a pass.

Sequence Diagram(s)

sequenceDiagram
  participant getShapeTransformCache
  participant tf2BufferCore
  participant tf2Buffer
  participant shapeTransformCache
  getShapeTransformCache->>tf2BufferCore: Request transform at timestamp
  tf2BufferCore-->>getShapeTransformCache: Return transform or failure
  getShapeTransformCache->>tf2Buffer: Retry with configured wait duration
  tf2Buffer-->>getShapeTransformCache: Return transform or failure
  getShapeTransformCache->>shapeTransformCache: Add entry when transform is available
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing redundant TF lookups from getShapeTransformCache.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@hugogo1998 hugogo1998 changed the title Drop the redundant canTransform in getShapeTransformCache Avoid redundant TF lookups in getShapeTransformCache Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant