Skip to content

Fix Lidar lag spike from RaycastLiDARSensor.cs#172

Merged
Autumn60 merged 1 commit intoField-Robotics-Japan:masterfrom
alexandrefch:fix/lidar_lag_spikes
Sep 28, 2024
Merged

Fix Lidar lag spike from RaycastLiDARSensor.cs#172
Autumn60 merged 1 commit intoField-Robotics-Japan:masterfrom
alexandrefch:fix/lidar_lag_spikes

Conversation

@alexandrefch
Copy link
Contributor

Fixed a performance bug when using lidar by changing the minimum raycast calculation per job.

As we see in the unity editor's profiling tool, there's a lag spike every time lidar run, and a single job seems to perform all the calculations.

Screenshot from 2024-09-12 15-58-44

After changing the minCommandsPerJob parameter from RaycastCommand.ScheduleBatch inside RaycastLiDARSensor.cs with a constant value of 256 (arbitrary value), we can see an improvement: there are no more lag peaks, the frame rate is now stable and tasks are better distributed between jobs.

after

@Autumn60 Autumn60 self-requested a review September 28, 2024 11:22
@Autumn60
Copy link
Contributor

So too much division of jobs is not good. 🤔
I learned a lot. Thanks!!

Copy link
Contributor

@Autumn60 Autumn60 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Fail of CI is due to the fact that it is a PR from a forked repository.

@Autumn60 Autumn60 merged commit d0c64ae into Field-Robotics-Japan:master Sep 28, 2024
@alexandrefch
Copy link
Contributor Author

So too much division of jobs is not good. 🤔 I learned a lot. Thanks!!

It's the opposite: before, all calculations were carried out on a single job; now, by giving job's tasks with 256 raycasts, calculations are parallel and equaly distributed between jobs.

@Autumn60
Copy link
Contributor

Understood.
I misunderstood that argument to be “number of jobs” instead of “number of commands per job”. 😆

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.

2 participants