stdio: optimize readln algorithm for non-char type

[ljmf00]

Instead of using buf.length = 0, use buf = buf[0..0] slice technique to clean
the array and reuse, if possible, the buffer to reduce GC allocations.

Signed-off-by: Luís Ferreira contact@lsferreira.net

---

Benchmark: https://gist.github.com/run-dlang/7d0a5264d186ac8db487447ec5352366

[dlang-bot]

Thanks for your pull request and interest in making D better, @ljmf00! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7681"

[ljmf00]

Force pushed due to codestyle issues.

[ljmf00]

Btw @wilzbach I think its better to discard approval states on active repositories when force push or new commits to avoid merge unwanted and unapproved changes.

[wilzbach]

I appreciate the concern, but I think it's fine here as there isn't so much traffic. We already have the bot automatically remove auto-merge from PRs so that it's not possible for an untrusted contributor to sneak code in.

[schveiguy]

Instead of using buf.length = 0, use buf = buf[0..0] slice technique to clean
the array and use an appender to do less GC allocations.

buf.length = 0 is identical to buf = buf[0 .. 0].

Not only that, but you are only doing that from within the case where s.length is 0. So you are not trying to reuse the buffer at all, right?

The original did not reuse the buffer (even though it claims to in the docs). Any speedups here are purely from using appender vs. the slower runtime append. Reusing the buffer should add a pretty significant improvement.

[ljmf00]

Instead of using buf.length = 0, use buf = buf[0..0] slice technique to clean
the array and use an appender to do less GC allocations.

buf.length = 0 is identical to buf = buf[0 .. 0].

Not only that, but you are only doing that from within the case where s.length is 0. So you are not trying to reuse the buffer at all, right?

buf.length = 0

uses core.internal.array.capacity._d_arraysetlengthTImpl runtime function template, which internally will basically do:

if (newlength <= (*p).length)
{
    *p = (*p)[0 .. newlength];
    void* newdata = (*p).ptr;
    return newdata[0 .. newlength];
}

which for 0 is basically buf = buf[0..0]. For the compiler this may be hard to optimize and at the end its the same.

The original did not reuse the buffer (even though it claims to in the docs). Any speedups here are purely from using appender vs. the slower runtime append. Reusing the buffer should add a pretty significant improvement.

Yes, you are right, I can reuse the existing buffer space instead of reallocate a new one. Done the changes, please review.

[ljmf00]

I appreciate the concern, but I think it's fine here as there isn't so much traffic. We already have the bot automatically remove auto-merge from PRs so that it's not possible for an untrusted contributor to sneak code in.

Ok, fair point 👍

[schveiguy]

uses core.internal.array.capacity._d_arraysetlengthTImpl runtime function template

ugh, you are right. When did this change? I thought setting array length to 0 was optimized, I don't think it used to call a runtime function at all.

If I use AST on run.dlang.io to give me the difference between setting length to 0 and slicing, it's... disturbing.

[schveiguy]

Looks good, and simpler. Nice!

[ljmf00]

ugh, you are right. When did this change? I thought setting array length to 0 was optimized, I don't think it used to call a runtime function at all.

If I use AST on run.dlang.io to give me the difference between setting length to 0 and slicing, it's... disturbing.

I made a PR to the compiler frontend to fix that dlang/dmd#11912 .