On 3/31/18 5:37 PM, Jonathan M Davis wrote:
> And every time you used another library, you'd have the same problem and
> have to add -unittest=- whatever for each and every one of them, or you
> would have to use -unittest= with everything from your application or
> library rather than using -unittest. I really don't see how that scales
> well, and it's way too manual and too easy to screw up. It might be a decent
> idea for a workaround, but it's not a great solution.

Nod. We really need to get to the point where the application of unittest is modular.

On Saturday, 31 March 2018 at 21:37:13 UTC, Jonathan M Davis wrote:
> On Saturday, March 31, 2018 08:28:31 Jonathan Marler via Digitalmars-d wrote:
>> On Friday, 30 March 2018 at 20:17:39 UTC, Andrei Alexandrescu
>>
>> wrote:
>> > On 3/30/18 12:12 PM, Atila Neves wrote:
>> >> Fast code fast, they said. It'll be fun, they said. Here's a D
>> >>
>> >> file:
>> >>      import std.path;
>> >>
>> >> Yep, that's all there is to it. Let's compile it on my laptop:
>> >>      /tmp % time dmd -c  foo.d
>> >>      dmd -c foo.d  0.12s user 0.02s system 98% cpu 0.139 total
>> >
>> > Could be faster.
>> >
>> >> That... doesn't seem too fast to me. But wait, there's more:
>> >>      /tmp % time dmd -c -unittest foo.d
>> >>      dmd -c -unittest foo.d  0.46s user 0.06s system 99% cpu
>> >>
>> >> 0.525 total
>> >
>> > Not fast. We need to make -unittest only affect the built module. Even though it breaks certain uses of __traits(getUnittests). No two ways about it. Who can work on that?
>> >
>> > Andrei
>>
>> If you approve of the -unittest=<pattern> approach then timotheecour has already offered to implement this.  It's pattern matching would work the same as -i and would also use the "implied standard exclusions" that -i uses, namely,
>>
>> -unittest=-std -unittest=-etc -unittest=-core
>>
>> This would mean that by default, just passing "-unittest" would exclude druntime/phobos just like "-i" by itself also does.
>
> And every time you used another library, you'd have the same problem and have to add -unittest=- whatever for each and every one of them, or you would have to use -unittest= with everything from your application or library rather than using -unittest. I really don't see how that scales well, and it's way too manual and too easy to screw up. It might be a decent idea for a workaround, but it's not a great solution.
>
> IMHO, this is really something that should be handled by the compiler. It simply shouldn't be compiling in the unittest blocks for modules that you're not compiling directly. And if that's done right, then this whole problem goes away without having to make sure that every project you work on is configured correctly to avoid pulling in the unit tests from everything that it depends on.
>
> And maybe figuring out what to do about __traits(getUnittests) complicates things, but it seems like the fact that we're even having this problem is due to a flaw in the design of D's unit tests and that that should be fixed, not worked around.
>
> - Jonathan M Davis

Let's make this conversation a bit more concrete, I'm not sure we are discussing the exact same thing.

The proposed solution is to have -unittest mean "compile unittests for all 'compiled modules' according to the pattern rules".  The default pattern rule is to include all modules except druntime/phobos.

Say you have two "packages" foo and bar that contain modules inside their respective directories.  With the proposed -unittest=<pattern> this is the semantics you would get.

dmd -unittest       foo/*.d bar/*.d     # compiles unittests for foo/*.d and bar/*.d
dmd -unittest=foo   foo/*.d bar/*.d     # compiles unittests for foo/*.d
dmd -unittest=bar   foo/*.d bar/*.d     # compiles unittests for bar/*.d
dmd -unittest=foo.x foo/*.d bar/*.d     # compiles unittests for foo/x.d
dmd -unittest=-bar  foo/*.d bar/*.d     # compiles unittests for foo/*.d

Note that the default behavior makes sense, but this mechanism also allows you more fine-graned control to limit unittesting to certain packages or modules.  This degree of control would be quite helpful to me, any of the previously listed use cases represent valid scenarios that I would like to be able to do.  Do you have another solution that would provide this functionality?  I don't see any reason not to support these use cases.

On Sunday, April 01, 2018 01:25:41 Jonathan Marler via Digitalmars-d wrote:
> On Saturday, 31 March 2018 at 21:37:13 UTC, Jonathan M Davis
>
> wrote:
> > On Saturday, March 31, 2018 08:28:31 Jonathan Marler via
> >
> > Digitalmars-d wrote:
> >> On Friday, 30 March 2018 at 20:17:39 UTC, Andrei Alexandrescu
> >>
> >> wrote:
> >> > On 3/30/18 12:12 PM, Atila Neves wrote:
> >> >> Fast code fast, they said. It'll be fun, they said. Here's a D
> >> >>
> >> >> file:
> >> >>      import std.path;
> >> >>
> >> >> Yep, that's all there is to it. Let's compile it on my
> >> >>
> >> >> laptop:
> >> >>      /tmp % time dmd -c  foo.d
> >> >>      dmd -c foo.d  0.12s user 0.02s system 98% cpu 0.139
> >> >>
> >> >> total
> >> >
> >> > Could be faster.
> >> >
> >> >> That... doesn't seem too fast to me. But wait, there's more:
> >> >>      /tmp % time dmd -c -unittest foo.d
> >> >>      dmd -c -unittest foo.d  0.46s user 0.06s system 99% cpu
> >> >>
> >> >> 0.525 total
> >> >
> >> > Not fast. We need to make -unittest only affect the built module. Even though it breaks certain uses of __traits(getUnittests). No two ways about it. Who can work on that?
> >> >
> >> > Andrei
> >>
> >> If you approve of the -unittest=<pattern> approach then timotheecour has already offered to implement this.  It's pattern matching would work the same as -i and would also use the "implied standard exclusions" that -i uses, namely,
> >>
> >> -unittest=-std -unittest=-etc -unittest=-core
> >>
> >> This would mean that by default, just passing "-unittest" would exclude druntime/phobos just like "-i" by itself also does.
> >
> > And every time you used another library, you'd have the same problem and have to add -unittest=- whatever for each and every one of them, or you would have to use -unittest= with everything from your application or library rather than using -unittest. I really don't see how that scales well, and it's way too manual and too easy to screw up. It might be a decent idea for a workaround, but it's not a great solution.
> >
> > IMHO, this is really something that should be handled by the compiler. It simply shouldn't be compiling in the unittest blocks for modules that you're not compiling directly. And if that's done right, then this whole problem goes away without having to make sure that every project you work on is configured correctly to avoid pulling in the unit tests from everything that it depends on.
> >
> > And maybe figuring out what to do about __traits(getUnittests) complicates things, but it seems like the fact that we're even having this problem is due to a flaw in the design of D's unit tests and that that should be fixed, not worked around.
> >
> > - Jonathan M Davis
>
> Let's make this conversation a bit more concrete, I'm not sure we are discussing the exact same thing.
>
> The proposed solution is to have -unittest mean "compile unittests for all 'compiled modules' according to the pattern rules".  The default pattern rule is to include all modules except druntime/phobos.
>
> Say you have two "packages" foo and bar that contain modules inside their respective directories.  With the proposed -unittest=<pattern> this is the semantics you would get.
>
> dmd -unittest       foo/*.d bar/*.d     # compiles unittests for
> foo/*.d and bar/*.d
> dmd -unittest=foo   foo/*.d bar/*.d     # compiles unittests for
> foo/*.d
> dmd -unittest=bar   foo/*.d bar/*.d     # compiles unittests for
> bar/*.d
> dmd -unittest=foo.x foo/*.d bar/*.d     # compiles unittests for
> foo/x.d
> dmd -unittest=-bar  foo/*.d bar/*.d     # compiles unittests for
> foo/*.d
>
> Note that the default behavior makes sense, but this mechanism also allows you more fine-graned control to limit unittesting to certain packages or modules.  This degree of control would be quite helpful to me, any of the previously listed use cases represent valid scenarios that I would like to be able to do.  Do you have another solution that would provide this functionality? I don't see any reason not to support these use cases.

My point is that this is not a good solution for the problem of dependencies having their unit tests included in your project. That should simply not be happening.

Discussions of ways to tell the compiler to enable unit tests for some modules and not others within your own project are IMHO therefore a separate issue. Now, as for the specific proposal trying to solve the issue of running only certain unit tests in your project, I think that it's overly complicated once you start having to deal with stuff like exclusions. I'd argue for simply compiling the module or modules that you care about with -unittest and that you not use -unittest when compiling the other modules. All of this is going to have to be controlled by whatever build system you're using anyway, since it really doesn't scale to keep calling the compiler directly like that, and once you're dealing with a build system like dub or make or whatever, then it can have support for something like

dub test --only=foo

where it then compiles only the module foo with -unittest, and compiles the rest with out -unittest. There shouldn't be any need to change dmd for that, just the build tool.

But regardless, I don't think that such a feature is a good solution for the problem of the unit tests from dependencies being compiled in. The compiler should be fixed to take care of that.

- Jonathan M Davis

On 3/30/2018 1:17 PM, Andrei Alexandrescu wrote:
> Could be faster.

It's been a fair amount of time since somebody has done profiling of dmd. It needs to be done. There's probably plenty of low hanging fruit. Speculating about why it is slow is pointless without data.

On Sunday, 1 April 2018 at 02:40:26 UTC, Walter Bright wrote:
> On 3/30/2018 1:17 PM, Andrei Alexandrescu wrote:
>> Could be faster.
>
> It's been a fair amount of time since somebody has done profiling of dmd. It needs to be done. There's probably plenty of low hanging fruit. Speculating about why it is slow is pointless without data.

have a look at https://github.com/dlang/dmd/pull/7792 for a little profiling utility which tells you about which parts of a programm draw compiletime.

Whenever I see long compile times 90% of it is due to templates.

On Friday, 30 March 2018 at 16:41:42 UTC, Jonathan Marler wrote:
>
> Seems like you're comparing apples to oranges.

No, I'm comparing one type of apple to another with regards to weight in my shopping bag before I've even taken a bite.

> Go's path.go is very small, a 215 line file:
> https://github.com/golang/go/blob/master/src/path/path.go
> Documentation: https://golang.org/pkg/path/

gocloc has it at 123 SLOC.

> Dlang's std.path is much more comprehensive with 4181 lines: https://github.com/dlang/phobos/blob/master/std/path.d
> Documentation: https://dlang.org/phobos/std_path.html

dscanner says 1857 SLOC. Also, that includes unit tests, of which there are 72 so probably some 700SLOC there.

I don't think how big the files are is revelant for me, a user of the standard library. If I want to do something with paths and don't want to roll my own code, I pay a price for it in D, whereas it's relatively free with Go. It makes me want to substitute every usage of std.path.buildPath in my code with just `foo ~ "/" ~ bar ~ ...`.

> It's over an order of magnitude more code

More lines of code is a liability, not an advantage.

> and only takes twice as long to compile without unittests,

No... that's ~11.583x with no unittests and ~43.75x with. The former number not being interesting to me in the slightest.

> and it's only fair to compare the "non-unittest" version of std.path with Go, since Go does not include unittests.

Absolutely not.

There is *0* compile-time penalty on Go programmers when they test their programs, whereas my compile times go up by a factor of 3 on a one-line program. And that's >3 multiplied by "already slow to begin with".

Atila

On Sunday, 1 April 2018 at 02:40:26 UTC, Walter Bright wrote:
> On 3/30/2018 1:17 PM, Andrei Alexandrescu wrote:
>> Could be faster.
>
> It's been a fair amount of time since somebody has done profiling of dmd. It needs to be done. There's probably plenty of low hanging fruit. Speculating about why it is slow is pointless without data.

I profiled it as soon as I had the numbers. I didn't bother to mention it because I thought I'd just work on making dmd faster instead.

I seem to be the one who feels the most pain by this, it'd be silly of me to expect anyone else to work on it.

On Monday, 2 April 2018 at 12:33:37 UTC, Atila Neves wrote:
> I don't think how big the files are is revelant for me, a user of the standard library. If I want to do something with paths and don't want to roll my own code, I pay a price for it in D, whereas it's relatively free with Go. It makes me want to substitute every usage of std.path.buildPath in my code with just `foo ~ "/" ~ bar ~ ...`.

That will also be _faster_ at runtime ;-)
std.path is a performance nightmare.
My favorite example is buildNormalizedPath:

https://github.com/dlang/phobos/blob/2e105c72a9e5fa028f31f1898ec8d479a9bae4a1/std/path.d#L1738

We ought to start to separate: the good, the bad and the ugly modules.

On Monday, 2 April 2018 at 12:35:03 UTC, Atila Neves wrote:
> On Sunday, 1 April 2018 at 02:40:26 UTC, Walter Bright wrote:
>> On 3/30/2018 1:17 PM, Andrei Alexandrescu wrote:
>>> Could be faster.
>>
>> It's been a fair amount of time since somebody has done profiling of dmd. It needs to be done. There's probably plenty of low hanging fruit. Speculating about why it is slow is pointless without data.
>
> I profiled it as soon as I had the numbers. I didn't bother to mention it because I thought I'd just work on making dmd faster instead.
>
> I seem to be the one who feels the most pain by this, it'd be silly of me to expect anyone else to work on it.

Making certain parts of the compiler faster is also on my agenda.
Feel free to talk to me anytime :)

On Mon, Apr 02, 2018 at 12:35:03PM +0000, Atila Neves via Digitalmars-d wrote:
> On Sunday, 1 April 2018 at 02:40:26 UTC, Walter Bright wrote:
> > On 3/30/2018 1:17 PM, Andrei Alexandrescu wrote:
> > > Could be faster.
> > 
> > It's been a fair amount of time since somebody has done profiling of dmd. It needs to be done. There's probably plenty of low hanging fruit.  Speculating about why it is slow is pointless without data.
> 
> I profiled it as soon as I had the numbers. I didn't bother to mention it because I thought I'd just work on making dmd faster instead.
> 
> I seem to be the one who feels the most pain by this, it'd be silly of me to expect anyone else to work on it.

Recently I've also started feeling the sting of slow compile times.  I don't know why I didn't notice it before... either I was jaded by C++ compile times in the past and even D's "slower" compile times are fast by comparison, or maybe dmd performance has degraded over time? It's a possibility we should investigate.  Or most likely, the recent templatization of certain parts of druntime and Phobos has exacerbated the problem to the point that it's now very noticeable.  The recent fiasco with __switch and `import std.format` come to mind.

While currently all the fingers seem to be pointing at templates, I have to say that I'm a big fan of templated code, and would rather see an improvement in how the compiler deals with templates, than a reduction in the usage of templates. Some of D's key selling features being templates and meta-programming, how they are implemented is pretty important.


T

-- 
Береги платье снову, а здоровье смолоду.