Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Lidar lag spike from RaycastLiDARSensor.cs #172

Merged

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
0 of 2 checks passed
@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