-
-
Notifications
You must be signed in to change notification settings - Fork 706
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 Issue 20566 - std.sformat should avoid allocating memory when pri… #7393
Conversation
|
Thanks for your pull request, @rainers! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + phobos#7393" |
|
I like your idea of having a fixed length buffer for the "small" numbers, which is replaced in case it doesn't fit by a gc allocated buffer. This won't produce wrong results, but should be faster and without gc for "small" numbers. The timing of this PR doesn't fit in my plans though. I'm currently waiting on PR #7264 to be merged. After that my plan was to add support for the %g qualifier, which I don't think is much of a problem as it's essentially deciding between %f and %e which are allready in there. Next I'd like to make the whole thing CTFEable. The PR for this is allready written for %e, %f and %a. Now your PR would interfere with this process and I would have to rewrite and test a lot of code and remove several merge conflicts. So I would be happy if this could be postponed for a while. After it's CTFEable I'm open for this sort of improvement. I allready though about using fixed length arrays for "smaller" numbers, but only for speed reasons, not for avoiding gc (I wasn't aware of this) and I had no idea how to mix this without code duplication, so I'm happy about your solution. And I also remember that I used |
Pedantically, no allocations should happen for std.sformat (see also https://issues.dlang.org/show_bug.cgi?id=13055), but correctly clipping the buffer would mean a very different way to write to the output. Some estimates for character length are not good enough for this.
I don't think merging will be very difficult, as most changes are adding
CTFEability would also be nice in druntime, as some function use sprintf there, too (e.g. core.demangle for float template arguments).
I think it might only make a difference if you iterate over a buffer with foreach, but I don't see this happening here. |
…nting floating point values use stack allocated buffers as long as the required precision is within reasonable values
|
Is this good to go? |
…nting floating point values
use stack allocated buffers as long as the required precision is within reasonable values