mjansen4857 / pathplanner

A simple yet powerful path planning tool for FRC robots
https://pathplanner.dev
MIT License
398 stars 125 forks source link

Use std::string_view instead of string when applicable to avoid heap allocations #683

Open r4stered opened 6 months ago

r4stered commented 6 months ago

For example, in NamedCommands, we pass an std::string, instead of a std::string_view. Passing a string_view would save some allocations.

mjansen4857 commented 1 month ago

So I looked into this a while back but I'm not sure I trust my own C++ knowledge enough to know where is safe to do so without the potential of problems. Especially with stuff like named commands where wpilib includes a map implementation that uses a string_view as a key. The actual string has to live somewhere and I'm not sure the what the best way to implement that is.

My approach at this point would probably just be to use string references for accessing maps and such, which should also avoid some allocations. But if you know what you're doing with respect to this string_view stuff, feel free to open a PR for it and I'll go for that over what I would do.

r4stered commented 1 month ago

At least in the case of NamedCommands, I've only ever seen them used with string literals, which will be around for the whole robot program. So creating a string_view from a literal is fine.

I am helping out the Choreo and PhotonVision team with their C++ vendorlib stuff currently, so if I have time I can take a look and make a PR :)