https://d.puremagic.com/issues/show_bug.cgi?id=11783



--- Comment #10 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-12-22 05:05:31 PST ---
FYI. I just tried commenting out all of the assertThrown unit tests in std.datetime, and the running time on my machine went from nearly 24 seconds to just under 4. So, while the other tests definitely need to be refactored and optimized (in which case, they'd take even less time), the ones that throw exceptions currently take up about 5/6th of the running time (and take up a far smaller portion than that of the total tests). So, optimizing those tests is definitely low hanging fruit.

If you'd like, since I do have a few days off for Christmas and can spend at least a little bit of time on D, I can spend some time over the next few days strategically disabling tests in order to improve how long std.datetime's tests take without losing a lot of tests, and then I can go back and refactor them properly in a month or two when I finally have time to focus on that. So, you'd get faster execution times but poorer testing in the short term, and then for the long term, I can sort out how to get good execution times without losing a lot of testing.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=11783


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com


--- Comment #11 from Walter Bright <bugzilla@digitalmars.com> 2013-12-22 11:00:48 PST ---
I want to chime in and agree that 38 seconds to unittest datetime is too slow. I want to add that unittesting datetime consumes more than 500Mb of memory (how much I don't know, but it exceeds all memory and fails on one of my older boxes).

It just seems patently absurd to me that date/time consumes more than 500Mb of memory.

I agree that the datetime tests should be thorough. But I see no reason why they can't be both thorough and run in a reasonable amount of time in a reasonable amount of space.

Let's assume that exceptions are the bottleneck. I posit that there's nothing about date/time calculations that require exceptions. I suggest that datetime separate out date/time calculations into separate nothrow functions, and have the throwing happen in a layer over the top of them. Then have the unittests test the non-throwing calculation engine, and you'll only need a handful to check to see that the throwing works.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=11783



--- Comment #12 from Andrei Alexandrescu <andrei@erdani.com> 2013-12-22 11:31:04 PST ---
(In reply to comment #9)
> I completely agree that 38 seconds is too long. That needs to be significantly reduced.

That's the spirit. Better yet, we should not have gotten to this point in the first place. That's 1.5 orders of magnitude beyond the "holy frak" moment.

> However, what I don't want to do is reduce it by simply reducing the
> number of tests. Experience has shown me (and other developers who I've talked
> to agree with me - e.g. Steven Schveighoffer who did the Tango time stuff years
> ago) that date/time-related tests need to be thorough because of all of the
> corner cases, many of which are hard to know because they depend on quirks of
> the calculations being done.

Though the assertions are true in principle, I completely disagree to where this has led std.datetime. This is not gene splitting or the Mars rover. There's just too much brute force testing exercising the same code paths over and over again, beyond all sense and reason. I think the "unittests are good" argument has been essentially used as an excuse to pile everything and anything on the account that more unittests lead to better unittesting.

> Maybe the number of tests will have to be reduced
> in order to get the time down, but if I can, I'd very much prefer to get the
> time down by optimizing the tests, which I believe can be done.

I think rethinking the approach to unittesting std.datetime from the ground up would be much better for everyone.

> Maybe I'm too touchy on this subject and come across badly, for which I'm sorry.

I'm sure I'm the more guilty party.

> But I believe very strongly that code in general (not just std.datetime) needs to have very thorough testing, and so I tend to object to attempts to reduce the number of tests of any code.

Again, this is a sensible statement but I think you have taken it to extremes that are wholly inappropriate. This is the main point I'm trying to get across with this bug report: just because X is good, one million X is not necessarily one million times (or any) better. We are contemplating a net and major negative gain here by overdoing what is fundamentally good.

(In reply to comment #10)
> FYI. I just tried commenting out all of the assertThrown unit tests in std.datetime, and the running time on my machine went from nearly 24 seconds to just under 4. So, while the other tests definitely need to be refactored and optimized (in which case, they'd take even less time), the ones that throw exceptions currently take up about 5/6th of the running time (and take up a far smaller portion than that of the total tests). So, optimizing those tests is definitely low hanging fruit.

Great, that's the spirit. I suggest you establish a budget - like 2 seconds - and stick with it. That's some 10 times more that what would be most appropriate, but still within reasonable bounds.

> If you'd like, since I do have a few days off for Christmas and can spend at least a little bit of time on D, I can spend some time over the next few days strategically disabling tests in order to improve how long std.datetime's tests take without losing a lot of tests, and then I can go back and refactor them properly in a month or two when I finally have time to focus on that. So, you'd get faster execution times but poorer testing in the short term, and then for the long term, I can sort out how to get good execution times without losing a lot of testing.

This is a volunteer project and it's not my place to tell anyone what to do and when. Family, school, work come first. I'm glad we got to the point of acknowledging the matter, and I want to make sure we don't repeat the mistake and that we consider it worth fixing.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=11783



--- Comment #13 from yebblies <yebblies@gmail.com> 2014-03-07 18:11:13 EST ---
It appears to be faster now.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=11783



--- Comment #14 from Jonathan M Davis <jmdavisProg@gmx.com> 2014-03-06 23:48:11 PST ---
> It appears to be faster now.

Fixing issue# 9584 removed the main bottlneck in the tests, so they're considerably faster now (currently under 3 seconds on my machine). They should still be refactored, but std.datetime is no longer the module with the slowest unit tests. From the looks of it, std.file now "wins" at around 16 seconds. I've been meaning for ages to rework those tests so that they're consolidated and therefore minimize the number of file operations it does to do the same tests (std.file is a case where putting each test after its corresponding function and having each of the tests be independent makes them take far longer than they would if we had one or two long unittest blocks that tested everything together), but it's a ways down on my todo list.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=11783



--- Comment #15 from Andrei Alexandrescu <andrei@erdani.com> 2014-03-07 09:24:34 PST ---
Thanks. Shall we close this now, or there's more to do?

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=11783


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


--- Comment #16 from Jonathan M Davis <jmdavisProg@gmx.com> 2014-03-07 11:55:23 PST ---
> Thanks. Shall we close this now, or there's more to do?

As the issue was that the std.datetime unit tests were taking too long, and now they're not, I think that we can close this. I'll be reworking them, which will affect how long they take, and I'll have to make sure that those changes don't reintroduce the problem, but I don't think that we need this bug report for that. That's code improvement but not really fixing a bug. At most, it simply risks reintroducing the problem of the tests taking too long if I screw up the changes. As it stands, they seem to be fine.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------