Currently, the only use of the internal FormatArg type is as the single argument to the call format!("{}", ...) inside the runtime_format! macro.
And the only reason FormatArg implements fmt::Display is to be able to be used in such a place.
IMO, it would be easier and more efficient just to have a function that builds the final String without all the fmt mechanisms. Except for the user-provided arguments, of course, that are still &dyn Display.
Since now we are writing into a String we can pre-allocate its memory, further improving the performance. I'm currently allocating twice the size of the format string, that it looks to me a nice sweet spot.
In this PR I've included two commits: the first one implements that change; the second one adds a benchmark (with crate criterion) to confirm the runtime improvements. In my machine I've measured an improvement between 10% and 25%.
I understand that you may prefer the old code over this hypothetical performance improvement. If so, feel free to disregard this PR, no worries. Also should you decide to merge it, you could remove the benchmark, if you think it is unnecessary, it is here mostly for show.
Usually performance in translations is not so important, but I'm using it in a Dear ImGui application, so I'm constantly translating strings, 60 whole UIs per second.
And with this one I reach the end of my PR spree. For now :smile:.
ogoffart
commented
3 months ago
Currently, the only use of the internal
FormatArgtype is as the single argument to the callformat!("{}", ...)inside theruntime_format!macro.And the only reason
FormatArgimplementsfmt::Displayis to be able to be used in such a place.IMO, it would be easier and more efficient just to have a function that builds the final
Stringwithout all thefmtmechanisms. Except for the user-provided arguments, of course, that are still&dyn Display.Since now we are writing into a
Stringwe can pre-allocate its memory, further improving the performance. I'm currently allocating twice the size of the format string, that it looks to me a nice sweet spot.In this PR I've included two commits: the first one implements that change; the second one adds a benchmark (with crate
criterion) to confirm the runtime improvements. In my machine I've measured an improvement between 10% and 25%.I understand that you may prefer the old code over this hypothetical performance improvement. If so, feel free to disregard this PR, no worries. Also should you decide to merge it, you could remove the benchmark, if you think it is unnecessary, it is here mostly for show.
Usually performance in translations is not so important, but I'm using it in a Dear ImGui application, so I'm constantly translating strings, 60 whole UIs per second.
And with this one I reach the end of my PR spree. For now :smile:.