On Saturday, 23 June 2018 at 20:17:01 UTC, Vladimir Panteleev
wrote:
On Friday, 22 June 2018 at 21:37:07 UTC, Walter Bright wrote:
text
begat textImpl
begat to
begat toImpl
begat toStr
begat formatValue
begat formatValueImpl
begat put
begat doPut
begat Appender.put
begat Appender.put
Hmm, this looks suboptimal for more than one reason. Wouldn't
`text` benefit from using an appender in its implementation,
rather than letting every invocation of `to` have its own? I.e.
there ought to be one Appender per `text` invocation, rather
than per one `text` argument.
textImpl seems to already have a branch for >1 arguments, but
it still invokes to!S(arg) for them.
`text` has an appender [0] but it calls `to!` for every
non-integer, so it has effectively O(n+1) GC allocations. We
should be able to get this down to O(1).
The initially obvious way to do this would be allowing `to!` to
take an `OutputRange`... but this would not improve the OP issue
Walter's had with trying to trace template expansions. (in fact
it would make it worse and add a lot of template bloat)
So my idea is to have a lower level API that is `@safe @nogc pure
nothrow`, takes already allocated memory and writes to it like:
---
// Reterning W[] here with a slice of the original buffer so you
can use it in tests easily.
@safe @nogc pure nothrow W[] writeCharsTo(T, W)(T input, scope
W[] buffer);
---
This could then be used in both `to!` and `text`, or directly if
the user wanted a @nogc option (ie json generator, or use with
std.io etc).
I've done some benchmarks with integers and `to!` becomes almost
twice as fast with `dmd -O -release -inline` and still ~30%
faster with `ldc2 -O3 -release` (as its zero copy unlike the
current `toChars` InputRange which copies when used in `to!`)
Early results of a `text` benchmark show even better speed
improvements.
I will clean up the benchmark code I have and post it for you to
see tonight.
In principle would this type of addition be accepted into phobos?
Thanks,
David
[0] https://github.com/dlang/phobos/blob/master/std/conv.d#L4243
