Add new flag -unittest-roots

[nordlow]

This speeds up running of unittests locally on my machine typically by a factor of 3x on my projects when incrementally making edits to a single module and wanting it to be self-checked on the fly by editor plugins such as FlyCheck. By its own unittests placed besides its declarations as it always should be. This until we have transitioned dmd into a compiler daemon listening for file system events like Zig now supports.

CLI-wise, alternatively we could instead adopt -unittest=roots or someting similar.

Works fine locally for me.

[dlang-bot]

Thanks for your pull request and interest in making D better, @nordlow! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#22906"

[dkorpel]

As always, compiler switches are bugs. Why can't this be the default behavior?

[nordlow]

As always, compiler switches are bugs.

Indeed.

Why can't this be the default behavior?

I've always wanted this to be the default behavior. I'm unsure about the motives for this not being the default. One can always get full unittest triggering via dub test.

[dkorpel]

I'm unsure about the motives for this not being the default

Me neither, I would try turning this on and see how the test suite reacts.

[kinke]

Unittests outside root modules shouldn't be parsed anymore already, since v2.099 (#13224):

dmd/compiler/src/dmd/parse.d

Lines 491 to 528 in cd441f7

	/**
	* Ignore unittests in non-root modules.
	*
	* This mainly means that unittests *inside templates* are only
	* ever instantiated if the module lexically declaring the
	* template is one of the root modules.
	*
	* E.g., compiling some project with `-unittest` does NOT
	* compile and later run any unittests in instantiations of
	* templates declared in other libraries.
	*
	* Declaring unittests *inside* templates is considered an anti-
	* pattern. In almost all cases, the unittests don't depend on
	* the template parameters, but instantiate the template with
	* fixed arguments (e.g., Nullable!T unittests instantiating
	* Nullable!int), so compiling and running identical tests for
	* each template instantiation is hardly desirable.
	* But adding a unittest right below some function being tested
	* is arguably good for locality, so unittests end up inside
	* templates.
	* To make sure a template's unittests are run, it should be
	* instantiated in the same module, e.g., some module-level
	* unittest.
	*
	* Another reason for ignoring unittests in templates from non-
	* root modules is for template codegen culling via
	* TemplateInstance.needsCodegen(). If the compiler decides not
	* to emit some Nullable!bool because there's an existing
	* instantiation in some non-root module, it has no idea whether
	* that module was compiled with -unittest too, and so whether
	* Nullable!int (instantiated in some unittest inside the
	* Nullable template) can be culled too. By ignoring unittests
	* in non-root modules, the compiler won't consider any
	* template instantiations in these unittests as candidates for
	* further codegen culling.
	*/
	// The isRoot check is here because it can change after parsing begins (see dmodule.d)
	if (doUnittests && mod.isRoot())

[kinke]

So the only change AFAICT here would be for -i, because of the earlier Module.isRoot() check (before parsing starts, see last comment line above), ignoring the unittests in such imported (and compiled) modules. So are you using -i for the cases where you see a ~3x speedup?

[nordlow]

So the only change AFAICT here would be for -i, because of the earlier Module.isRoot() check (before parsing starts, see last comment line above), ignoring the unittests in such imported (and compiled) modules. So are you using -i for the cases where you see a ~3x speedup?

Yes, I have been using -i all along. You can easily observe the change in behavior via

./test.sh 
2 modules passed unittests
1 modules passed unittests

inside of

https://codeberg.org/nordlow/d-snippets/src/branch/main/unittest-roots

when using the nordlow:unittest-roots branch.

[kinke]

Okay. - Well -i is incrementally-unfriendly as it gets, totally wasteful. So for quick minimal incremental builds after a change, reggae works best (implicitly using -makedeps etc.) for the unittest config. The runtime part, only running the tests of the single module of interest, would require tests filtering (via CLI flags for the unittest runner executable), which is doable with unit-threaded for example.

So I don't think we should special-case -unittest -i (edit: let alone introducing an extra CLI switch), NOT parsing the unittests in modules dragged in via -i, which are otherwise treated as regular root modules everywhere else.

[nordlow]

All code comments addressed.

[nordlow]

So I don't think we should special-case -unittest -i (edit: let alone introducing an extra CLI switch), NOT parsing the unittests in modules dragged in via -i, which are otherwise treated as regular root modules everywhere else.

Does this mean that you are ok with changing the default behavior of -unittest -i to NOT include unittests of modules (dragged by the -i switch) not present on the command line? As proposed at #22906 (comment).

[kinke]

No, absolutely not. I don't think we should encourage any -i -unittest (clearly a 'poor man' solution/hack for inadequate IDE tooling), so not changing the compiler (or adding a new feature/CLI option) to accommodate for such usages.

[nordlow]

No, absolutely not. I don't think we should encourage any -i -unittest (clearly a 'poor man' solution/hack for inadequate IDE tooling), so not changing the compiler (or adding a new feature/CLI option) to accommodate for such usages.

Are you saying we should expect users to solely depend on using reggae (or dub or redub) in their workflow to get these speed gains? I want an effective solution that solely requires the compiler itself regardless of whether we consider this a hack or not. This is the direction that Zig is taking and I believe it's the right one.

I'm gonna try enabling this behavior by default and see what CI says.

The real issue I want to address here is that there currently is no way of selectively enabling/disabling unittests at the module level other than separate compilation of source files which in turns means redundant (re)parsing (and partially also semantic analysis) of shared imported modules.

[kinke]

I'm not sure you (and Nick, Dennis etc.) knew that unittests in non-root modules are already ignored (not parsed). You are solely tackling the -unittest -i effect of including the unittests of the imported (and compiled) modules too, which some users might not want (but some might, to check all unittests of all dragged-in modules at once). I think understand that sentiment, but adding a CLI option which only affects the -i -unittest combo makes IMO no sense, provided that we all agree that -i -unittest is a terrible combo (but maybe enough for some users, either because their projects are tiny, or their acceptance of overly long build times is higher than mine).

-i is just terrible in general for any kind of incremental builds. It always rebuilds all dragged-in modules (with druntime and Phobos modules as sole exceptions), and that serially, in a single compiler process, single-threaded. It doesn't get any more terrible.

So I'm sorry, but I really think the proposed feature is bad, because it encourages the terrible combo. And in case DMD ever becomes a daemon, we surely won't use anything like -i to rebuild everything every time. Until then, fast incremental D builds are something for a designated build system like ninja, and according build generators like reggae. We added -makedeps etc. to integrate D compilers rather easily in the existing build systems for C++ and such.

[kinke]

The real issue I want to address here is that there currently is no way of selectively enabling/disabling unittests at the module level other than separate compilation of source files which in turns means redundant (re)parsing (and partially also semantic analysis) of shared imported modules.

Then I suggest working on the default unittest runner implementation (edit:

dmd/druntime/src/core/runtime.d

Line 561 in 16f3201

extern (C) UnitTestResult runModuleUnitTests()

) instead, making it work like the druntime/Phobos unittest runners - supporting an optional list of modules to be tested as CLI options for the runner:

dmd/druntime/src/test_runner.d

Lines 25 to 48 in 16f3201

	return Runtime.args.length > 1 ? testModules() : testAll();
	}
	string mode;
	UnitTestResult testModules()
	{
	UnitTestResult ret;
	ret.summarize = false;
	ret.runMain = false;
	foreach (name; Runtime.args[1..$])
	{
	immutable pkg = ".package";
	immutable pkgLen = pkg.length;
	if (name.length > pkgLen && name[$ - pkgLen .. $] == pkg)
	name = name[0 .. $ - pkgLen];
	doTest(getModuleInfo(name), ret);
	}
	return ret;
	}

[nordlow]

Then I suggest working on the default unittest runner implementation (edit:

I mean selectively disable their compilation. Sorry for being imprecise.

[kinke]

Well the point is that other modules shouldn't be compiled at all if they (and their imports) haven't changed, not just skipping (compiling and later running) their unittests.

[dkorpel]

I'm not sure you (and Nick, Dennis etc.) knew that unittests in non-root modules are already ignored (not parsed).

Good you bring that up, I didn't know that. I also assumed that modules found by -i are identical to explicitly passed .d files (so also 'root' internally). But this PR strips unittests before -i dependencies are discovered, that's different than what I originally thought this PR was doing.

adding a CLI option which only affects the -i -unittest combo makes IMO no sense

Agreed

provided that we all agree that -i -unittest is a terrible combo

Disagreed, it's my go to for running tests. But if I want to run only a subset of unittests, I agree it's up to the test runner to filter.

What I have been considering is -unittest=<pattern> using the exact same mechanism as -i=<pattern>. I think in a DLF meeting we might have discussed doing -preview=xxx=<pattern> at some point, it's a general problem that command line switches also affect third-party code that's hard to fix. I'd be in favor of doing that.

[nordlow]

Well the point is that other modules shouldn't be compiled at all if they (and their imports) haven't changed, not just skipping (compiling and later running) their unittests.

Yes, and that solely roles on a build tool until the compiler has support for caching which Walter has been reluctant to adding to the compiler itself despite this is what most compilers of modern AOT-compiled languages now support, like Go, Rust, and Zig.

[kinke]

provided that we all agree that -i -unittest is a terrible combo

Disagreed, it's my go to for running tests.

Oh wow, do you not use dub then for the projects you are testing? I've just run a little comparison, for a dub module itself, simulating working on source/dub/commandline.d (the first module I've found with a bunch of unittests) and wanting to run its tests. Using DMD v2.112.0 on my Ubuntu 24 laptop, using lld v18 as default linker; timings are best-of-3:

Manual approach using dmd -unittest -i:

# cmdline flags derived from a `dub test -v` run, painful:
$ /usr/bin/time -v dmd -g -unittest -i -Isource/ -I../../.dub/cache/dub/1.42.0-beta.1+commit.6.g62bf8ed1/code/dub-test-library-unittest-QHCgb3T3k9vRtLm7-qEAeA/ -version=DubUseCurl -main -run source/dub/commandline.d
[…]
	Percent of CPU this job got: 109%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:06.12
	Maximum resident set size (kbytes): 4285564

Using reggae:

# generate ninja build:
$ reggae

# first unittest runner build:
$ /usr/bin/time -v ninja ut
[…]
	Percent of CPU this job got: 911%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:03.36
	Maximum resident set size (kbytes): 1662620

# incrementally rebuild it after touching the file:
$ touch source/dub/commandline.d && /usr/bin/time -v ninja ut
[…]
	Percent of CPU this job got: 169%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:02.33
	Maximum resident set size (kbytes): 1534440

# run the tests (all dub unittests, not just the single `dub.commandline` module, but no unittests from further dub dependencies etc.):
$ /usr/bin/time -v bin/dub-test-library
[…]
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.11
	Maximum resident set size (kbytes): 19028

With reggae --per-module, to compile each module separately:

$ reggae --per-module
$ /usr/bin/time -v ninja ut
[…]
	Percent of CPU this job got: 1631%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:06.58
	Maximum resident set size (kbytes): 949244
$ touch source/dub/commandline.d && /usr/bin/time -v ninja ut
[…]
	Percent of CPU this job got: 243%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:01.54
	Maximum resident set size (kbytes): 859036

So here in this case, an incremental --per-module reggae build takes ~1.5 secs (edit: 0.4 secs of which for linking), and then 0.1 secs for running the unittests of 59 modules. Building and running the tests via dmd -unittest -i -main -run takes 6.1 secs, i.e., about 4 times as long. And this for the dub unittest runner itself, which has no dependencies on other dub packages, unlike most real-world code, so for more complex projects with more dependencies, the benefit should be significantly higher.

[dkorpel]

Oh wow, do you not use dub then for the projects you are testing?

At SARC we use dub, Bastiaan set up the build system there. For dmd and Phobos when I have specific unittests to run, I often use a oneliner like dmd -i=std -c -unittest -version=StdUnittest -preview=dip1000 std/package.d. For my own code I use a makefile + dmd -i -makedeps and a custom light druntime (adapted from arsd/webassembly). Compiling everything takes about 1 second, but dub takes 2.

dmd build/test.d -g -debug -i (...) 1.05s user 0.06s system 95% cpu 1.166 total

dub build --config=unittest  1.86s user 0.30s system 96% cpu 2.246 total

That's 86 KLOC for dmd -i to compile (based on a wc -l of the makedeps). Compiling a single module and its dependencies (instead of all 300) typically takes 0.3 seconds, but it obviously depends on how much that specific module imports.

dub compiles/analyzes more LOC (it imports std.exception for one) but that's exactly the problem: by default it puts everything in the source folder on the command line, while -i lazily picks what's needed. I haven't tried reggae yet, but with my current 1s compile times there's not much incentive to switch build system, but I might try it out nonetheless out of curiosity.

[kinke]

Yeah, the potentially extra needed cmdline flags (at least the -I import roots for other dub package dependencies) are IMO already enough of a hurdle to make -unittest -i an exotic manual use case (certainly with a few legitimate use cases), mostly for simple cases/projects, only depending on druntime/Phobos etc. Certainly not suited for generic IDE tooling because of that, unless the IDE integration uses dub as a library itself (like reggae). And for reggae, we could e.g. easily add a generic run-ut target (like ut), to build+run the tests, which currently requires knowledge of the path to the generated unittest runner executable. Then it should be rather straight-forward to use reggae+ninja for an IDE tool, finally making parallelized incremental builds the default D dev experience.

Too bad your day-to-day builds are so fast already. :D - Reggae can really make a huge difference for large projects, trust me, we have proven this at Symmetry. :)

[dkorpel]

Too bad your day-to-day builds are so fast already. :D - Reggae can really make a huge difference for large projects, trust me, we have proven this at Symmetry. :)

I trust you :) but my own strategy is generally to avoid large projects and only depend on a few important libs from the linux package manager. I should probably try reggae at SARC though, dub build times are often 20-30 seconds for me.

[kinke]

Okay, the 20-30 secs sound more interesting - yeah, please give it a try and let us know then. :) - As shown in the example, the usage is trivial - I recommend simply running reggae in the dub project root dir (the one with dub.sdl/dub.json), once, to generate the ninja build. (See --dub-config, --dub-build-type and --dub-deps-objs for customization.) You can e.g. use a prebuilt reggae executable bundled with LDC (as long as DMD is activated, it will still use it by default). Then a dub build is basically a ninja invocation (building the default ninja target), dub test is ninja ut + running the generated unittest runner manually (=> ninja run-ut if we had it). Edit: And e.g. compiling both in parallel would be ninja default ut.