On Mon, Feb 05, 2018 at 01:27:57PM -0800, H. S. Teoh via Digitalmars-d wrote:
> One of my D projects for the past while has been taking unusually long times to compile.  This morning, I finally decided to sit down and figure out exactly why.
[...]

I don't want this thread to turn into ragging on std.regex, so here's the next instalment of this saga.

After taking std.regex out of the equation, I found that there was still something else that was bottlenecking my builds.  So I went for another hunt, and discovered another guilty party: std.format.

----
import std.format;
void main() { format("a"); }
----

Here I found something interesting: compiling with -unittest takes over 0.8s. But compiling *without* -unittest takes only 0.4s.

The compilation times of std.format itself aside (it's pretty heavyweight because of heavy template usage, and the recent compile-time format string checking, while pretty cool functionally, also comes at the price of heavier CTFE, i.e., slower compilation -- I'd like to leave digging into std.format for later), it's a little disturbing that compiling with -unittest *doubled* the compilation time.

This reinforces a concern I've been having recently, that is, library unittests that get instantiated when user code is compiled with -unittest.  I'll be the first to say that having built-in unittests in D has revolutionized the way I write code -- it has dramatically increased the average quality of my code. But its current simplicity comes at a cost: if you're using external libraries (i.e., outside the immediate codebase you're working with, so that includes Phobos), compiling with -unittest automatically inherits all unittests from all libraries that you import, even if said unittests have nothing to do with your own code.

Put another way: why should you be concerned with Phobos unittests when you're building your own code?  Phobos unittesting should have been (and is) already done by the autotester and CIs on github; there's no reason these tests have to be run over and over again in user code.

So I'd like to propose that we do something similar to what we did with template instantiations a couple of years ago: make it so that unittests are only instantiated if the module they occur in is being compiled, otherwise ignore them (even in the face of -unittest).  This way, adding unittests to Phobos won't cause unintentional slowdowns / unittest bloat across *all* D projects that import the affected Phobos modules. (Seen from this angle, it's a pretty hefty cost.)

Of course, this statement has to be qualified a bit: it's probably still a good idea to instantiate unittests inside templates -- since ostensibly they could be sanity-checking specific instantiations of templates. Though I'd argue that 90% of the time, their location inside a template is a mistake; they really should be outside templates and only instantiated once, because generally they test specific instantiations of the template rather than implement generic tests that apply across all template instantiations.


T

-- 
Never step over a puddle, always step around it. Chances are that whatever made it is still dripping.

On Wednesday, 7 February 2018 at 01:20:04 UTC, H. S. Teoh wrote:
> So I'd like to propose that we do something similar to what we did with template instantiations a couple of years ago: make it so that unittests are only instantiated if the module they occur in is being compiled, otherwise ignore them (even in the face of -unittest).  This way, adding unittests to Phobos won't cause unintentional slowdowns / unittest bloat across *all* D projects that import the affected Phobos modules. (Seen from this angle, it's a pretty hefty cost.)
>

Would it help to take the approach of mir, i.e. putting version(mir_test) before all the unittests?

On Tuesday, February 06, 2018 17:20:04 H. S. Teoh via Digitalmars-d wrote:
> So I'd like to propose that we do something similar to what we did with template instantiations a couple of years ago: make it so that unittests are only instantiated if the module they occur in is being compiled, otherwise ignore them (even in the face of -unittest).  This way, adding unittests to Phobos won't cause unintentional slowdowns / unittest bloat across *all* D projects that import the affected Phobos modules. (Seen from this angle, it's a pretty hefty cost.)

Well, that sounds like a DIP is in order.

> Of course, this statement has to be qualified a bit: it's probably still a good idea to instantiate unittests inside templates -- since ostensibly they could be sanity-checking specific instantiations of templates. Though I'd argue that 90% of the time, their location inside a template is a mistake; they really should be outside templates and only instantiated once, because generally they test specific instantiations of the template rather than implement generic tests that apply across all template instantiations.

I agree that most of the time, unittest blocks inside templates shouldn't be compiled with the template and that that mistake is made _way_ too often (even in Phobos), but once in a while, it's actually desirable. I created a DIP previously for using static to indicate that a unittest block inside a template should not be treated as part of the template, but I haven't moved it from the wiki to github, and Andrei pretty much told me that he though that such a DIP was a waste of time and that I shouldn't continue with it. He seems to think that the solutions involving using static ifs on all of the unittest blocks and compiling them in with a specific instantiation only are perfectly reasonable and that it's not worth adding anything to the language for it. I don't agree, but I have no idea how to convince him.

Regardless, because some unittest blocks really should be compiled in with every instantiation of a template (even if it's rare), I think that it's clear that unittest blocks that are instantiated with a template should be included in a program so long as any module that is specifically compiled instantiates the template. It may be that the implementation would require that they be there regardless, but even if that's true, not including the non-templated unittest blocks which are not explicitly compiled in would definitely be an improvement.

- Jonathan M Davis

On Wednesday, February 07, 2018 01:47:19 jmh530 via Digitalmars-d wrote:
> On Wednesday, 7 February 2018 at 01:20:04 UTC, H. S. Teoh wrote:
> > So I'd like to propose that we do something similar to what we did with template instantiations a couple of years ago: make it so that unittests are only instantiated if the module they occur in is being compiled, otherwise ignore them (even in the face of -unittest).  This way, adding unittests to Phobos won't cause unintentional slowdowns / unittest bloat across *all* D projects that import the affected Phobos modules. (Seen from this angle, it's a pretty hefty cost.)
>
> Would it help to take the approach of mir, i.e. putting
> version(mir_test) before all the unittests?

It would, but if we have to do that in front of all unittest blocks in a library, I think that that's a strong sign that the current design has a serious problem that should be fixed if possible.

- Jonathan M Davis

how about using same syntax (and reusing logic) as newly introduced:
` -i=+foo.bar,+baz-baz.bad`

`dmd -unittest=+foo.bar,+baz,-baz.bad rest_of_arguments`

which would only enable unittests as specified? It's flexible and intuitive

On Tue, Feb 6, 2018 at 5:56 PM, Jonathan M Davis via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> On Wednesday, February 07, 2018 01:47:19 jmh530 via Digitalmars-d wrote:
>> On Wednesday, 7 February 2018 at 01:20:04 UTC, H. S. Teoh wrote:
>> > So I'd like to propose that we do something similar to what we did with template instantiations a couple of years ago: make it so that unittests are only instantiated if the module they occur in is being compiled, otherwise ignore them (even in the face of -unittest).  This way, adding unittests to Phobos won't cause unintentional slowdowns / unittest bloat across *all* D projects that import the affected Phobos modules. (Seen from this angle, it's a pretty hefty cost.)
>>
>> Would it help to take the approach of mir, i.e. putting
>> version(mir_test) before all the unittests?
>
> It would, but if we have to do that in front of all unittest blocks in a library, I think that that's a strong sign that the current design has a serious problem that should be fixed if possible.
>
> - Jonathan M Davis
>

On Wednesday, 7 February 2018 at 01:20:04 UTC, H. S. Teoh wrote:
> On Mon, Feb 05, 2018 at 01:27:57PM -0800, H. S. Teoh via Digitalmars-d wrote:
>> [...]
> [...]
>
> I don't want this thread to turn into ragging on std.regex, so here's the next instalment of this saga.
>
> [...]

Already done. There's `version(StdUnittest)` since a few days:

https://github.com/dlang/phobos/pull/5927
https://github.com/dlang/phobos/pull/6107

On Wednesday, 7 February 2018 at 02:05:46 UTC, Timothee Cour wrote:
> how about using same syntax (and reusing logic) as newly introduced:
> ` -i=+foo.bar,+baz-baz.bad`
>
> `dmd -unittest=+foo.bar,+baz,-baz.bad rest_of_arguments`
>
> which would only enable unittests as specified? It's flexible and intuitive

I like it! That only leaves a question of defaults.

Yes. This has been a personal annoyance for many years. Even tried arguing some time back to get it fixed to no avail. Really hoping for better success this time.

On 02/06/2018 08:47 PM, jmh530 wrote:
> 
> Would it help to take the approach of mir, i.e. putting version(mir_test) before all the unittests?

That used to be a very common idiom. (And I agree with Jonathan M Davis: it's a STRONG sign the current design needs fixed). But newer versions of dub, intelligently, will only compile the files in the main project with -unittest, which solves the problem...*for dub users*. Unfortunately, this means that the idiom above has become less common and libraries have become less usable for anyone using a build tool *other* than dub. :(

On Wed, Feb 07, 2018 at 10:59:51PM -0500, Nick Sabalausky (Abscissa) via Digitalmars-d wrote:
[...]
> On 02/06/2018 08:47 PM, jmh530 wrote:
> > 
> > Would it help to take the approach of mir, i.e. putting
> > version(mir_test) before all the unittests?
> 
> That used to be a very common idiom. (And I agree with Jonathan M Davis: it's a STRONG sign the current design needs fixed). But newer versions of dub, intelligently, will only compile the files in the main project with -unittest, which solves the problem...*for dub users*. Unfortunately, this means that the idiom above has become less common and libraries have become less usable for anyone using a build tool *other* than dub. :(

Actually, it does *not* solve the problem for dub users.  Compiling with -unittest causes unittests for imported modules to be instantiated too. (Otherwise, Phobos unittests wouldn't show up in a -unittest build since we never compile Phobos modules directly in a user project!)


T

-- 
ASCII stupid question, getty stupid ANSI.

On Wednesday, 7 February 2018 at 08:21:01 UTC, Seb wrote:
> There's `version(StdUnittest)` since a few days

And I've just submitted a pull request that adds `version(StdUnittest)` to every unittest in the standard library.

https://github.com/dlang/phobos/pull/6159