Speed up shadow filter - Githubissues

ros-perception / laser_filters

Assorted filters designed to operate on 2D planar laser scanners, which use the sensor_msgs/LaserScan type.

BSD 3-Clause "New" or "Revised" License

172 stars 204 forks source link

Speed up shadow filter #144

Closed erwinbonsmatopic closed 2 years ago

erwinbonsmatopic commented 2 years ago

Improve performance of scan shadow filter.

Changes:

Instead of calculating sine/cosine each update, store (few) needed values in look-up tables and re-use those
Remove use of set collection (and associated dynamic memory allocations) in update method
Minor improvements to for-loop end criteria and checks inside loops

Together this results in speed-up of update method by more than factor three.

Added test to verify that lookup arrays is updated when needed. Updated detector test to include negative angles (and more generally, match usage of detector in filter).

erwinbonsmatopic commented 2 years ago

@jonbinney Can you have a look at this?

jonbinney commented 2 years ago

Thanks, this looks almost ready to merge! Two more requests:

could you add comments before each of the isShadow methods in the header, describing when to use each?
i think the use of private and friend class here is unneeded complexity, and others may want to use the new, faster method. Could you remove the private and friend stuff and make the new isShadow public?

erwinbonsmatopic commented 2 years ago

Thanks, this looks almost ready to merge! Two more requests:

could you add comments before each of the isShadow methods in the header, describing when to use each?

i think the use of private and friend class here is unneeded complexity, and others may want to use the new, faster method. Could you remove the private and friend stuff and make the new isShadow public?

Code reworked as requested.

jonbinney commented 2 years ago

Oops, sorry, one last thing to change: could you remove the empty desctructor for the detector class?

erwinbonsmatopic commented 2 years ago

Oops, sorry, one last thing to change: could you remove the empty desctructor for the detector class?

Done. Also removed the lingering friend declaration that was not needed anymore.

jonbinney commented 2 years ago

Thanks for all your work on this - it is a big improvement in speed!