toString refactor in druntime

[Steven Schveighoffer via Digitalmars-d]

On 10/27/14 8:01 PM, Manu via Digitalmars-d wrote:
>   28 October 2014 04:40, Benjamin Thaut via Digitalmars-d
> <digitalmars-d at puremagic.com> wrote:
>> Am 27.10.2014 11:07, schrieb Daniel Murphy:
>>
>>> "Benjamin Thaut"  wrote in message news:m2kt16$2566$1 at digitalmars.com...
>>>>
>>>> I'm planning on doing a pull request for druntime which rewrites every
>>>> toString function within druntime to use the new sink signature. That
>>>> way druntime would cause a lot less allocations which end up beeing
>>>> garbage right away. Are there any objections against doing so? Any
>>>> reasons why such a pull request would not get accepted?
>>>
>>>
>>> How ugly is it going to be, since druntime can't use std.format?
>>
>>
>> They wouldn't get any uglier than they already are, because the current
>> toString functions within druntime also can't use std.format.
>>
>> An example would be to toString function of TypInfo_StaticArray:
>>
>> override string toString() const
>> {
>>          SizeStringBuff tmpBuff = void;
>>          return value.toString() ~ "[" ~
>> cast(string)len.sizeToTempString(tmpBuff) ~ "]";
>> }
>>
>> Would be replaced by:
>>
>> override void toString(void delegate(const(char)[]) sink) const
>> {
>>          SizeStringBuff tmpBuff = void;
>>          value.toString(sink);
>>          sink("[");
>>          sink(cast(string)len.sizeToTempString(tmpBuff));
>>          sink("]");
>> }
>
> The thing that really worries me about this synk API is that your code
> here produces (at least) 4 calls to a delegate. That's a lot of
> indirect function calling, which can be a severe performance hazard on
> some systems.
> We're trading out garbage for low-level performance hazards, which may
> imply a reduction in portability.

I think given the circumstances, we are better off. But when we find a 
platform that does perform worse, we can try and implement alternatives. 
I don't want to destroy performance on the platforms we *do* support, 
for the worry that some future platform isn't as friendly to this method.

> But in any case, I think all synk code like this should aim to call
> the user supplied synk delegate at most *once* per toString.
> I'd like to see code that used the stack to compose the string
> locally, then feed it through to the supplied synk delegate in fewer
> (or one) calls.

This is a good goal to have, regardless. The stack is always pretty high 
performing. However, it doesn't scale well. If you look above, the 
function already uses the stack to output the number. It would be 
trivial to add 2 chars to put the "[]" there also so only one sink call 
occurs.

But an aggregate which relies on members to output themselves is going 
to have a tough time following this model. Only at the lowest levels can 
we enforce such a rule.

Another thing to think about is that the inliner can potentially get rid 
of the cost of delegate calls.

> Ideally, I guess I'd prefer to see an overload which receives a slice
> to write to instead and do away with the delegate call. Particularly
> in druntime, where API and potential platform portability decisions
> should be *super*conservative.

This puts the burden on the caller to ensure enough space is allocated. 
Or you have to reenter the function to finish up the output. Neither of 
these seem like acceptable drawbacks.

What would you propose for such a mechanism? Maybe I'm not thinking of 
your ideal API.

-Steve