Conversation
BaseStore had no direct test coverage; its lookups were only exercised through the engine-backed common tests. Add a unit test with a minimal object type that checks every object stays reachable by index, ID, name and pointer across additions, duplicate rejection, each removal accessor, DestroyAll and RemoveAll. IDs are fed in decreasing order (as BaseScene::CreateObjectId assigns them), decreasing with wrap-around past zero, increasing, and unordered. Assisted-by: Claude Opus 5 Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
GetById, ContainsId, RemoveById and DestroyById resolved IDs with a linear scan over the store. The ogre2 thermal, GPU rays, segmentation and bounding box material switchers call Scene::VisualById for every Ogre item on every rendered pass (GPU rays once per cubemap face), so the per-frame cost grows with the square of the scene size. Objects created through BaseScene get IDs from a decrementing counter and stores only append or erase, so the store vector is normally ordered by (first ID - ID). ConstIterById now bisects on that key, which stays ordered across unsigned wrap-around, and falls back to the previous linear scan when the bisection misses. Stores holding caller-chosen IDs in any other order return the same results as before. RemoveImpl walked the whole name map on every removal, including when removing the last element. Erase by the object's name and skip the re-indexing at the tail, which makes DestroyAll linear. No data members or signatures change, so the change is ABI compatible and can be backported. Measured with the real header and a heap-scattered mock object at -O2, with IDs assigned the way BaseScene::CreateObjectId does: case before after one GetById per object, N=4096 5.6 ms 0.29 ms one GetById per object, N=16384 152 ms 1.8 ms DestroyAll, N=16384 476 ms 2.0 ms Stores filled with increasing caller-chosen IDs keep the previous lookup cost. Assisted-by: Opencode (Union Alpha Free), Claude Opus 5 Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
|
Asked the agents to turn this into a real experiment that demonstrate how the gain is tangible, the results makes sense to me like nothing is gained in small worlds but there are important gains in large worlds. Take it with a grain of salt (mostly vibe-coded experiment) or double verify the numbers under your own experiment: The gain depends on scene size. In gz-sim's stock tunnel world there is no measurable change. With the tunnel's models repeated 16 times (2,226 models, 9,534 visuals), the lidar scans 2.1× faster, and 2.5× faster when the robot's four RGBD cameras also run. Setup: the installed gz-sim 11 nightly, run headless with ogre2. Both builds are gz-rendering 9cd640c (the commit the nightly was built from) and differ only in BaseStorage.hh from aad8d1e. Every run confirmed from the process's memory map that it loaded the libraries under test. The robot is the Explorer R2 from Fuel: a 360° lidar at 15 Hz and four RGBD cameras at 30 Hz. The ×4 and ×16 worlds repeat every model 3 km apart, so the robot sees the same tunnel while the scene grows. Physics ran as fast as possible, and I alternated which build ran first. Results are medians of 5 runs (3 for lidar + cameras). │ World │ Visuals │ Sensors │ Before (ms per scan) │ After (ms per scan) │ Gain │ In practice, one simulated minute in the ×16 world with all sensors went from 21.8 to 8.6 minutes of wall time. Load time stays at 74 s with either build. Why ×4 gains nothing but ×16 gains 2×: a scan takes as long as the slower of two threads, because the simulation thread waits for the previous render before handing over new poses.
|
2 participants
🦟 Bug fix
Summary
Doing some experiments with agents locally, one of them found a performance problem in the way we are handling BaseStore IDs
GetById, ContainsId, RemoveById and DestroyById resolved IDs with a linear scan over the store. The ogre2 thermal, GPU rays, segmentation and bounding box material switchers call Scene::VisualById for every Ogre item on every rendered pass (GPU rays once per cubemap face), so the per-frame cost grows with the square of the scene size.
Objects created through BaseScene get IDs from a decrementing counter and stores only append or erase, so the store vector is normally ordered by (first ID - ID). PR changes: ConstIterById now bisects on that key, which stays ordered across unsigned wrap-around, and falls back to the previous linear scan when the bisection misses. Stores holding caller-chosen IDs in any other order return the same results as before.
RemoveImpl walked the whole name map on every removal, including when removing the last element. Erase by the object's name and skip the re-indexing at the tail, which makes DestroyAll linear.
Numbers
Measured with the real header and a heap-scattered mock object at -O2, with IDs assigned the way BaseScene::CreateObjectId does:
case before after
one GetById per object, N=4096 5.6 ms 0.29 ms
one GetById per object, N=16384 152 ms 1.8 ms
DestroyAll, N=16384 476 ms 2.0 ms
Stores filled with increasing caller-chosen IDs keep the previous lookup cost.
Tests
The PR includes a test that verifies how the IDs are handled by the BaseScene so we have some warning if we are breaking one of the considered use cases.
Backport Policy
Checklist
codecheckpassed (See contributing)Assisted-by: Opencode (Union Alpha Free), Claude Opus 5
Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-byandGenerated-bymessages.Backports: If this is a backport, please use Rebase and Merge instead.