phobos
phobos copied to clipboard
Draft: Reduce template bloat in std.format
This, for instance, reduces memory usage of unittest-checking std.format.write
as
dmd -o- -vcolumns -wi -debug -unittest -dip25 -dip1000 -version\=StdUnittest -I/home/per/Work/phobos/ /home/per/Work/phobos/std/format/write.d
from 386 MB to 356 MB.
- Motive of added const qualifers is to have, for instance,
format(fmt, int.init)
format(fmt, const(int).init)
format(fmt, immutable(int).init)
...
all result in the same template instance of format
.
-
I've factored out integer printing to make the code not be instantiated for each integral type potentially reducing generated code a lot when formatting and printing integrals. Separate PR at https://github.com/dlang/phobos/pull/8278.
-
The refactoring in
formatValue
avoids the need to instantiate the wrapper templatesBooleanTypeOf
,IntegralTypeOf
, etc.
I'm gonna look into other types classes such as floating point types next.
Putting this out into the open to see what CIs feel about this.
Feel free to tell me how you want this split up.
Thanks for your pull request and interest in making D better, @nordlow! 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:
andReturns:
)
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#8247"
Irrespective of the memory gains, I think that having the parameter to format being const
makes sense on its own as I cannot imagine any scenario where the argument might get modified.
As of 68f0445c2 this PR so far reduces number of instances of formatValue
from 10890 to 5459 in make -f posix.mak BUILD=debug unittest
. More to come.
Just uncomment line 1233 in std/fomat/write.d and see for yourself.
FYI, @RazvanN7 @andralex @kinke
@nordlow this is currently failing the testing pipeline.
@nordlow this is currently failing the testing pipeline.
Thanks. I'm gonna branch of minor changes from this. So I'm gonna wait with fixing this.
This PR currently has two issue that blocks a complete reduction of code bloat in template-variadic formatting and printing functions like std.format.format
. These are
auto format(T...)(const(T) args) {}
fails when any of the args
is a struct with at least one field being a const class
(which is related to the lack of expressing head-mutable class fields in D without resorting to the Rebindable
-hack which, btw, contamintes logic in hasAliasing
). There is currently no test for this case in Phobos so those need to be added aswell.
auto format(T...)(const T args) {}
fails when any of the args
is an InputRange
. We might be able to define two overloads of formatValue
; one taking a mutable T
when isInputRange!(T)
and one taking a const T arg
when !isInputRange!(T)
.
These two issues also holds for std.format.text()
, write()
, writeln()
, and trace...
functions in std.experimental.logging
which are subject to the same parameter qualification as format
.
Afaict, both are related to lack of
- language support for expressing tail-const (head-mutable) pointer and class fields and
- implicit conversion of a struct S from const to tail-const (head-mutable) when S has any
const
field being either pointer or class. @andralex has previously eluded to that those rules are subject to relaxation.
I'm personally not sure which alternative is the best long-term.
FYI, @adamdruppe @maxhaton.
@nordlow I don't know of any plans to introduce tail-constness into the language, so I suggest, in the interest of progress to implement in this PR whatever works and is not dependent on tail-constness. Otherwise, this will be stalled forever.