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?

Kind Regards
Benjamin Thaut

"Benjamin Thaut"  wrote in message news:m2kt16$2566$1@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?

Am 27.10.2014 11:07, schrieb Daniel Murphy:
> "Benjamin Thaut"  wrote in message news:m2kt16$2566$1@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 advantage would be that the new version now ideally never allocates. While the old version allocated 3 times of which 2 allocations end up beeing garbage right away.

Also I rember reading that the long term goal is to convert all toString functions to the sink version.

Kind Regards
Benjamin Thaut

"Benjamin Thaut"  wrote in message news:m2m3j2$ciu$1@digitalmars.com...

> 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 advantage would be that the new version now ideally never allocates. While the old version allocated 3 times of which 2 allocations end up beeing garbage right away.
>
> Also I rember reading that the long term goal is to convert all toString functions to the sink version.

It's very ugly compared to the formattedWrite version, and it does increase the line count compared to the current version (this is the main disadvantage of the sink-based toString IMO).  If this is as bad as it gets, PR approval shouldn't be a problem. 

On 10/27/14 2:45 PM, Daniel Murphy wrote:
> "Benjamin Thaut"  wrote in message news:m2m3j2$ciu$1@digitalmars.com...
>
>> 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 advantage would be that the new version now ideally never
>> allocates. While the old version allocated 3 times of which 2
>> allocations end up beeing garbage right away.
>>
>> Also I rember reading that the long term goal is to convert all
>> toString functions to the sink version.
>
> It's very ugly compared to the formattedWrite version, and it does
> increase the line count compared to the current version (this is the
> main disadvantage of the sink-based toString IMO).  If this is as bad as
> it gets, PR approval shouldn't be a problem.

Might I suggest a helper that takes a sink and a variadic list of strings, and outputs those strings to the sink in order.

-Steve

On Mon, 27 Oct 2014 15:20:24 -0400
Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com>
wrote:

> Might I suggest a helper that takes a sink and a variadic list of strings, and outputs those strings to the sink in order.
hehe. simple CTFE writef seems to be a perfect fit for druntime. i implemented very "barebone", yet powerful PoC in ~18Kb of code (~500 lines). it can be made even smaller if necessary. then one can use something like `writef!"<%s:%5s>"(sink, msg, code);` and so on.

On 10/27/14 4:24 PM, ketmar via Digitalmars-d wrote:
> On Mon, 27 Oct 2014 15:20:24 -0400
> Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com>
> wrote:
>
>> Might I suggest a helper that takes a sink and a variadic list of
>> strings, and outputs those strings to the sink in order.
> hehe. simple CTFE writef seems to be a perfect fit for druntime. i
> implemented very "barebone", yet powerful PoC in ~18Kb of code (~500
> lines). it can be made even smaller if necessary. then one can use
> something like `writef!"<%s:%5s>"(sink, msg, code);` and so on.
>

I think this is overkill for this purpose. We need something simple to save a few lines of code.

-Steve

On Mon, 27 Oct 2014 17:04:55 -0400
Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com>
wrote:

> I think this is overkill for this purpose. We need something simple to save a few lines of code.
18KB (even less) module which consists mostly of functional templates, generates nice string mixin and adds part of writef functionality and can convert nicely formatted strings and args to series of calls to any function is overkill? ok. who i am to teach people that metaprogramming is the way to automate boring things...

sure, i won't force anyone to use that, i can't take the pleasure of writing sink boilerplate calls from people.

 28 October 2014 04:40, Benjamin Thaut via Digitalmars-d
<digitalmars-d@puremagic.com> wrote:
> Am 27.10.2014 11:07, schrieb Daniel Murphy:
>
>> "Benjamin Thaut"  wrote in message news:m2kt16$2566$1@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 wonder if the trade-off between flexibility and performance went a
little too far towards flexibility in this case. It's better, but I
don't think it hits the mark, and I'm not sure that hitting the mark
on both issues is impossible.

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.

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.

> The advantage would be that the new version now ideally never allocates. While the old version allocated 3 times of which 2 allocations end up beeing garbage right away.
>
> Also I rember reading that the long term goal is to convert all toString functions to the sink version.
>
> Kind Regards
> Benjamin Thaut

On 10/27/14 6:02 PM, ketmar via Digitalmars-d wrote:
> On Mon, 27 Oct 2014 17:04:55 -0400
> Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com>
> wrote:
>
>> I think this is overkill for this purpose. We need something simple
>> to save a few lines of code.
> 18KB (even less) module which consists mostly of functional templates,
> generates nice string mixin and adds part of writef functionality and
> can convert nicely formatted strings and args to series of calls to any
> function is overkill? ok. who i am to teach people that metaprogramming
> is the way to automate boring things...

Meta has a cost with the current compiler. It would be nice if it didn't, but I have practical concerns.

> sure, i won't force anyone to use that, i can't take the pleasure of
> writing sink boilerplate calls from people.

I think a few simple functions can suffice for druntime's purposes. We don't need a kitchen sink function (pun intended).

-Steve