| Message ID | 20210118061853.26808-1-maxim.cournoyer@gmail.com |
|---|---|
| Headers | show |
Hello! Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > > [...] > >> I never felt the need for this since most individual files run quickly >> enough (and those that don’t should be optimized…), but it can be >> useful. > > What triggered it for me was trying to iterate using tests added to the > tests/packages.scm test module: [...] > 1.6 s; better than 46 s! Quite a difference, indeed! Some tests, like ‘tests/store.scm’, invoke the GC (via ‘delete-paths’ calls in particular), and this is a bad idea because it takes ages. What I meant by “should be optimized” is that these tests should be tweaked to avoid invoking the GC as much as possible. That’s not the case of ‘tests/packages.scm’ though. Which gives me an idea: what would it take to modify the test driver so it can print the time spent in each test? :-) That would probably prove helpful to optimize core Guix. In many cases, I also run the one test I’m interested in through Geiser, which gives an optimally fast feedback loop. > We can also check the time the suspected slow test took: > > $ time make check TESTS=tests/packages.scm SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without cache'" > [...] > PASS: tests/packages.scm - fold-available-packages with/without cache Ah ha! Turns out a large part of the time was due to the O(n²) behavior of the various list operations (the list of packages is big enough!). Fixed in 73744725dd0a65cddaa9251f104f17ca27756479. >>> +The underlying SRFI 64 custom Automake test driver used for the 'check' >>> +test suite (located at @file{build-aux/test-driver.scm}) also allows >> >> Maybe shorten to “The underlying test driver (located at >> @file{build-aux/test-driver.scm}) also allows”. > > I see value in explicitly stating what it is, as it took me some effort > to be able to answer that question when I started looking at it (the > test driver). Agreed. It just seemed to me that we were mentioning three new concepts/tools in passing: SRFI-64, Automake, and test drivers. > I've now pushed this series to master; thank you for the review! Thank you! Ludo’.
Hi Ludo, Ludovic Courtès <ludo@gnu.org> writes: > Hello! > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> Ludovic Courtès <ludo@gnu.org> writes: >> >> [...] >> >>> I never felt the need for this since most individual files run quickly >>> enough (and those that don’t should be optimized…), but it can be >>> useful. >> >> What triggered it for me was trying to iterate using tests added to the >> tests/packages.scm test module: > > [...] > >> 1.6 s; better than 46 s! > > Quite a difference, indeed! > > Some tests, like ‘tests/store.scm’, invoke the GC (via ‘delete-paths’ > calls in particular), and this is a bad idea because it takes ages. > What I meant by “should be optimized” is that these tests should be > tweaked to avoid invoking the GC as much as possible. > > That’s not the case of ‘tests/packages.scm’ though. Which gives me an > idea: what would it take to modify the test driver so it can print the > time spent in each test? :-) > > That would probably prove helpful to optimize core Guix. Good idea! I've pushed 5e652e94a9 that does this! Here's a sample, showing the three slowest test cases of the tests/packages.scm test: $ export SCM_LOG_DRIVER_FLAGS="--brief=no --show-duration=yes" $ make check TESTS=tests/packages.scm --8<---------------cut here---------------start------------->8--- [...] PASS: tests/packages.scm - fold-available-packages with/without cache [17.121s] PASS: tests/packages.scm - find-packages-by-name [0.000s] PASS: tests/packages.scm - find-packages-by-name with version [0.000s] PASS: tests/packages.scm - find-packages-by-name with cache [9.492s] PASS: tests/packages.scm - find-packages-by-name + version, with cache [7.412s] [...] PASS: tests/packages.scm - find-package-locations with cache [7.484s] PASS: tests/packages.scm - specification->location [0.000s] --8<---------------cut here---------------end--------------->8--- Automake could be extended to support this extra data; it would be interesting to be able to set the number of slowest tests to display, such as DURATIONS=5, to show only the 5 slowest tests, for example. > In many cases, I also run the one test I’m interested in through Geiser, > which gives an optimally fast feedback loop. I've done that too, although it's somewhat clunky to use: the test modules name not being mapped to an actual filename hierarchy trips the module loading mechanism of Guile/Geiser. I usually fix that such that (test-packages) -> (tests packages), then I can load the module normally. Except that's not what I want to do as it runs all the tests. So I must copy paste the module declaration to the REPL to get the symbols imported, then the other required definitions and the (test-begin... due to SRFI 64 stateful design. Not my idea of a sleek test feedback loop, coming from pytest and other modern test frameworks; still very powerful to be able to do everything at the REPL though! >> We can also check the time the suspected slow test took: >> >> $ time make check TESTS=tests/packages.scm >> SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without >> cache'" >> [...] >> PASS: tests/packages.scm - fold-available-packages with/without cache > > Ah ha! Turns out a large part of the time was due to the O(n²) behavior > of the various list operations (the list of packages is big enough!). > Fixed in 73744725dd0a65cddaa9251f104f17ca27756479. Woohoo! That looks clever! Too clever for me to understand in the 2 minutes I starred at it ;-). That test now runs almost 6x faster (~8 s on a fast machine). Well done! >>>> +The underlying SRFI 64 custom Automake test driver used for the 'check' >>>> +test suite (located at @file{build-aux/test-driver.scm}) also allows >>> >>> Maybe shorten to “The underlying test driver (located at >>> @file{build-aux/test-driver.scm}) also allows”. >> >> I see value in explicitly stating what it is, as it took me some effort >> to be able to answer that question when I started looking at it (the >> test driver). > > Agreed. It just seemed to me that we were mentioning three new > concepts/tools in passing: SRFI-64, Automake, and test drivers. Yeah, it's a mouthful, for better or worse. Thank you, Maxim
Hi, On Tue, 02 Feb 2021 at 00:47, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > $ export SCM_LOG_DRIVER_FLAGS="--brief=no --show-duration=yes" > $ make check TESTS=tests/packages.scm > > --8<---------------cut here---------------start------------->8--- > [...] > PASS: tests/packages.scm - fold-available-packages with/without cache [17.121s] > PASS: tests/packages.scm - find-packages-by-name [0.000s] > PASS: tests/packages.scm - find-packages-by-name with version [0.000s] > PASS: tests/packages.scm - find-packages-by-name with cache [9.492s] > PASS: tests/packages.scm - find-packages-by-name + version, with cache [7.412s] > [...] > PASS: tests/packages.scm - find-package-locations with cache [7.484s] > PASS: tests/packages.scm - specification->location [0.000s] > --8<---------------cut here---------------end--------------->8--- Cool! Thanks Maxim. I find it really better. :-) >>> We can also check the time the suspected slow test took: >>> >>> $ time make check TESTS=tests/packages.scm >>> SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without >>> cache'" >>> [...] >>> PASS: tests/packages.scm - fold-available-packages with/without cache >> >> Ah ha! Turns out a large part of the time was due to the O(n²) behavior >> of the various list operations (the list of packages is big enough!). >> Fixed in 73744725dd0a65cddaa9251f104f17ca27756479. Neat! Cool! Cheers, simon