On 04/02/2018 08:35 AM, 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.

A large and obvious time sink is that unittests in library code are built when user code is to be unittested. I'd recommend doing this before any other optimization.

On Mon, Apr 02, 2018 at 12:09:01PM -0400, Andrei Alexandrescu via Digitalmars-d wrote:
> On 04/02/2018 08:35 AM, 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.
> 
> A large and obvious time sink is that unittests in library code are built when user code is to be unittested. I'd recommend doing this before any other optimization.

Lately this has been mentioned more and more frequently.  So what's the status on this?  Are we going to move forward with making it so that -unittest only applies to modules supplied on the command-line?

Has there been an investigation into how __traits(getUnitTests) could be made to work in spite of this change?


T

-- 
Indifference will certainly be the downfall of mankind, but who cares? -- Miquel van Smoorenburg

On 04/02/2018 12:22 PM, H. S. Teoh wrote:
> Lately this has been mentioned more and more frequently.  So what's the
> status on this?  Are we going to move forward with making it so that
> -unittest only applies to modules supplied on the command-line?
> 
> Has there been an investigation into how __traits(getUnitTests) could be
> made to work in spite of this change?

Walter and I are willing to go with the change assuming no showstopper presents itself, but we don't have time budgeted for it. So it needs a strong champion.

On Monday, 2 April 2018 at 12:33:37 UTC, Atila Neves wrote:
> 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.

My point was that GO's path library is very different from dlang's std.path library.  It has an order of magnitude less code so the point was that you're comparing a very small library with much less functionality to a very large one.

>> It's over an order of magnitude more code
>
> More lines of code is a liability, not an advantage.

I didn't say anything about whether it was advantageous, the point was that it's more code so you should take that into account when you evaluate performance.  You're post was misleading because it was assuming that both libraries were comparable when in reality they appear to be very different.


>> 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".

My point was that if you want to compare "compile-time" performance, you should not include the unittests in D's time since Go does not include unittests.  In practicality, D should not be compiling in the standard library unittest by default.  This is a problem that should be fixed but still doesn't change the fact that not taking this into consideration would be an unfair comparison.

On Monday, April 02, 2018 18:52:14 Jonathan Marler via Digitalmars-d wrote:
> On Monday, 2 April 2018 at 12:33:37 UTC, Atila Neves wrote:
> > 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.
>
> My point was that GO's path library is very different from dlang's std.path library.  It has an order of magnitude less code so the point was that you're comparing a very small library with much less functionality to a very large one.
>
> >> It's over an order of magnitude more code
> >
> > More lines of code is a liability, not an advantage.
>
> I didn't say anything about whether it was advantageous, the point was that it's more code so you should take that into account when you evaluate performance.  You're post was misleading because it was assuming that both libraries were comparable when in reality they appear to be very different.
>
> >> 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".
>
> My point was that if you want to compare "compile-time" performance, you should not include the unittests in D's time since Go does not include unittests.  In practicality, D should not be compiling in the standard library unittest by default. This is a problem that should be fixed but still doesn't change the fact that not taking this into consideration would be an unfair comparison.

You both have good points. On the one hand, yes, std.path is doing more, so it's not surprising that it takes longer to compile, and in that sense, it's comparing apples and oranges. However, from the standpoint of the user, they're just calling these functions to get something done, and the implementation details don't really matter. So, from the user's standpoint, as far as compilation time goes, std.path is just worse. The reasons why are kind of irrelevant from that perspective. So, in a way, you're both right.

Now, ultimately, given how D works and the functionality in std.path, I don't know that it really makes sense for it to compile as fast as Go's solution if Go's solution is doing so much less, but regardless, we should be trying to at least eliminate the unnecessary slowdowns for things like compiling unit tests, and we should be looking into how to speed up the stuff that's slow to compile (e.g. improving the compilation speed of templates in general would be a huge boon). std.path doesn't need to be as slow to compile as it is to do what it does.

- Jonathan M Davis

On 4/2/18 8:33 AM, Atila Neves wrote:

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

Here is an interesting tidbit as well -- importing with -unittest takes 4x as long AND NO CODE IS GENERATED OR WILL EVER BE GENERATED. Literally, the compiler is parsing, semantically analyzing, consuming 300% more cycles just to "compile" code it will never actually use or generate files for, and as far as I know, it has no way of doing so.

I did a simple test:

dmd -c -v testpath.d | grep import | wc -l
60
dmd -c -v -unittest testpath.d | grep import | wc -l
87

So there are 27 more modules processed by DMD that make this 300% increase happen (Not to mention, ~60 imports for building paths...). Doing some bash-script work, I get the following differences

> import    core.atomic
> import    core.bitop
> import    core.checkedint
> import    core.math
> import    core.stdc.fenv
> import    core.stdc.math
> import    core.sync.exception
> import    core.sync.mutex
> import    core.sys.darwin.mach.port
> import    core.sys.darwin.mach.thread_act
> import    core.sys.darwin.pthread
> import    core.sys.posix.pthread
> import    core.sys.posix.sched
> import    core.sys.posix.semaphore
> import    core.sys.posix.stdlib
> import    core.sys.posix.sys.wait
> import    core.thread
> import    std.algorithm
> import    std.algorithm.internal
> import    std.algorithm.setops
> import    std.algorithm.sorting
> import    std.bitmanip
> import    std.math
> import    std.process
> import    std.random
> import    std.system
> import    std.utf

One disconcerting thing here is, not all of these imports come from std.path. Some of them come from unit tests in modules imported by std.path unittests. Because inside a unit test you want to have free access to all of phobos to do whatever the hell you want for testing, it's quite easy to import ALL of phobos when you do unit tests.

We really need to change the unittest import strategy.

-Steve

On 4/2/18 12:28 PM, Andrei Alexandrescu wrote:
> On 04/02/2018 12:22 PM, H. S. Teoh wrote:
>> Lately this has been mentioned more and more frequently.  So what's the
>> status on this?  Are we going to move forward with making it so that
>> -unittest only applies to modules supplied on the command-line?
>>
>> Has there been an investigation into how __traits(getUnitTests) could be
>> made to work in spite of this change?
> 
> Walter and I are willing to go with the change assuming no showstopper presents itself, but we don't have time budgeted for it. So it needs a strong champion.

If nobody else volunteers, I might give it a shot at the hackathon. I'd need help figuring out where the important bits are, so I may be bugging compiler devs that day :)

-Steve

On Mon, Apr 02, 2018 at 03:28:02PM -0400, Steven Schveighoffer via Digitalmars-d wrote: [...]
> We really need to change the unittest import strategy.
[...]

I think this has been established beyond reasonable doubt for the last little while.  What about we start hashing out a solution?

AFAIK, the current proposal is to make it so that `-unittest` only takes effect on modules that are being compiled (i.e., specified on the command-line).  Imported modules will *not* have unittests compiled, unless they have also been specified on the command-line.

The only thing blocking this proposal that I'm aware of, is that it will break __traits(getUnitTests).  But I'm not sure if that's actually a serious problem at all.  What if we make it so that a unittest block in an imported module that isn't specified on the command-line is basically treated as a template? I.e., parse the AST, but don't do anything with it.  Don't bother running semantic, don't bother resolving identifiers (in particular, import statements inside the unittest), etc.. Just leave it in the AST as essentially translated syntax.  Then if __traits(getUnitTests) is ever invoked, run semantic on the unittests in the targeted module.

I'm not 100% certain, but I have a suspicion that this will mitigate the breakage to __traits(getUnitTests) without compromising on the "don't compile unittests outside the current list of modules to compile" fix.


T

-- 
Questions are the beginning of intelligence, but the fear of God is the beginning of wisdom.

On Monday, April 02, 2018 12:55:05 H. S. Teoh via Digitalmars-d wrote:
> On Mon, Apr 02, 2018 at 03:28:02PM -0400, Steven Schveighoffer via Digitalmars-d wrote: [...]
>
> > We really need to change the unittest import strategy.
>
> [...]
>
> I think this has been established beyond reasonable doubt for the last little while.  What about we start hashing out a solution?
>
> AFAIK, the current proposal is to make it so that `-unittest` only takes effect on modules that are being compiled (i.e., specified on the command-line).  Imported modules will *not* have unittests compiled, unless they have also been specified on the command-line.
>
> The only thing blocking this proposal that I'm aware of, is that it will break __traits(getUnitTests).  But I'm not sure if that's actually a serious problem at all.  What if we make it so that a unittest block in an imported module that isn't specified on the command-line is basically treated as a template? I.e., parse the AST, but don't do anything with it.  Don't bother running semantic, don't bother resolving identifiers (in particular, import statements inside the unittest), etc.. Just leave it in the AST as essentially translated syntax.  Then if __traits(getUnitTests) is ever invoked, run semantic on the unittests in the targeted module.
>
> I'm not 100% certain, but I have a suspicion that this will mitigate the breakage to __traits(getUnitTests) without compromising on the "don't compile unittests outside the current list of modules to compile" fix.

Having never used __traits(getUnitTest), I'm not very familiar with it, but depending on what it does, it might be enough to register the fact that a unittest block exists in the file, and then the unittest block itself only needs to be analyzed enough to parse passed it. But I don't know. I'll have to study up on what it does exactly to say much intelligent about it.

One concern I have is version(unittest) blocks. In order to avoid code breakage, those would need to still be compiled in. I know that I've personally used version(unittest) blocks that had package access level and were then imported in the unit tests within that package in order to avoid declaring helper types or functions in each module. So, treating version(unittest) blocks like they're not there when importing a module would definitely break som existing code. It could be argued that such uses should be deprecated in some manner, but simply not compiling in anything related to -unittest in imported modules would be a problem. However, aside from maybe __traits(getUnitTests), I don't see why it would be a problem to ignore code inside a unittest block except for when that module is being compiled (or when that unittest block is inside a template that's being compiled).

- Jonathan M Davis

On 4/2/18 3:55 PM, H. S. Teoh wrote:
> On Mon, Apr 02, 2018 at 03:28:02PM -0400, Steven Schveighoffer via Digitalmars-d wrote:
> [...]
>> We really need to change the unittest import strategy.
> [...]
> 
> I think this has been established beyond reasonable doubt for the last
> little while.  What about we start hashing out a solution?
> 
> AFAIK, the current proposal is to make it so that `-unittest` only takes
> effect on modules that are being compiled (i.e., specified on the
> command-line).  Imported modules will *not* have unittests compiled,
> unless they have also been specified on the command-line.
> 
> The only thing blocking this proposal that I'm aware of, is that it will
> break __traits(getUnitTests).  But I'm not sure if that's actually a
> serious problem at all.  What if we make it so that a unittest block in
> an imported module that isn't specified on the command-line is basically
> treated as a template? I.e., parse the AST, but don't do anything with
> it.  Don't bother running semantic, don't bother resolving identifiers
> (in particular, import statements inside the unittest), etc.. Just leave
> it in the AST as essentially translated syntax.  Then if
> __traits(getUnitTests) is ever invoked, run semantic on the unittests in
> the targeted module.
> 
> I'm not 100% certain, but I have a suspicion that this will mitigate the
> breakage to __traits(getUnitTests) without compromising on the "don't
> compile unittests outside the current list of modules to compile" fix.

Yes, I think this approach (at least, parsing unittests but deferring semantic until invoked) is the correct way to avoid breakage. It may be easy enough to do this without "treating it as a template", but I'm not sure.

-Steve