otland
Use string views to avoid copying strings
Pull Request Prelude
- [x] I have followed proper The Forgotten Server code styling.
- [x] I have read and understood the contribution guidelines before making this PR.
- [x] I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.
Changes Proposed
Avoids some string copies by using string_view to wrap existing strings instead of copying to a new string instance. Most notably, avoids copying strings from database column names and query results, and avoids copying strings when reading prop streams.
Additionally, throws in a bit of code modernization by replacing output parameters with returning pairs and destructuring assignments.
Issues addressed: Closes #4064
Any server owner with a relatively large server willing to try and share the results? Can't properly measure with local benchmarks :sweat_smile:
If we rely on fmt instead of c++ 20 formatting api, we can use fmt::memory_buffer buffer + fmt::format_to + string_view to prevent copies in queries and other stuff. Here a really small optimization based on this concept:
fmt::memory_buffer buffer;
const ItemType& it = Item::items[item->getID()];
if (!it.showCount) {
fmt::format_to(std::back_inserter(buffer), FMT_COMPILE("Using one of {:s}..."), item->getName());
} else if (count == 1) {
fmt::format_to(std::back_inserter(buffer), FMT_COMPILE("Using the last {:s}..."), item->getName());
} else {
fmt::format_to(std::back_inserter(buffer), FMT_COMPILE("Using one of {:d} {:s}..."), count, item->getPluralName());
}
player->sendTextMessage(MESSAGE_HOTKEY_PRESSED, std::string_view(buffer.data(), buffer.size()));
This is a "avoid" copying the final string but even more importantly allocation at all costs approach. The fmt::memory_buffer is a 500 bytes stack allocated buffer that is used internally by fmt::format function and here we are using it directly to avoid the copy (and extra allocations if the final string is bigger than 500 bytes) from the stack to a new allocated in the heap string which would happen if we used fmt::format function.
@yamaken93 this is a good idea in general, but I would be careful not to overuse it
@yamaken93 this is a good idea in general, but I would be careful not to overuse it
I agree, its a "war" on allocation + copy idea. Can be used for strings retrieved from the lua API(when lua calls TFS C functions), strings from the protocol, etc.
@EPuncker @ramon-bernardo may I get an updated review? Rebased and fixed a conflict