Speed up compilation in Clang's "PerformPendingInstallations" step

[claremacrae]

Approval Tests instantiates a lot of things every time you #include ApprovalTests.hpp, instead of just once (typically in the main).

Figure out and implement a pattern so that the PerformPendingInstallations (which is the Purple step in chrome://tracing view, when built with clang-9 and -ftime-trace) are only done in the main.cpp and not re-done in every test file.

Catch2's mechanism will work for us.

[claremacrae]

To get tracing files out of Clang 9 or above, add this to CMakeLists.txt:

    if(CMAKE_CXX_COMPILER_ID MATCHES ".*Clang" AND 
            CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL 9.0)
        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ftime-trace")
    endif()

[claremacrae]

This effectively means splitting out the definitions of at least the slow-to-compile code from the declarations - so that the slow-to-compile code is only compiled once, in the main, and not in all the test files as well.

[claremacrae]

Use cases:

People who have...

• downloaded a released single-header file and added it to their builds

People who have cloned or forked our repo, and...

• are compiling their own code against the individual headers
• are compiling their own code against the simulated single header
• are developing our code, i.e. us, or people creating Pull Requests

Build configurations

• Unity builds (which give duplicate symbol errors for symbols in Scrubbers.cpp)

[claremacrae]

I did a little experimenting with separate out the Scrubbers.h function implementations to Scrubbers.cpp...

Things to remember when we do this for real:

1. We'll need to change ApprovalTests/CMakeLists.cpp so that most INTERFACE are PUBLIC and the .cpp files are listed (I have a shelved change)
2. When creating the .cpp file, it will be worth starting it with this content... CLion uses the namespace block if it's there, but doesn't add one itself....

#include "Blah.h"

namespace ApprovalTests
{

}

One reason is that there will be a lot less manual editing of return types needed this way, as CLion's refactoring doesn't add the ApprovalTests:: to return types - plus, it will save ApprovalTests:: being added before every function's name, which will shorten lines and add readability.

3. We will probably want to script the creation of all the .cpp files alongside their .h files - and then later delete ones that have no code in. Reasons:

   • Creating each of them one-by-one and is very tedious, and causes a lot of waiting for CMake to re-run
   • Some of our .h files don't include all the other .h files that they depend on, e.g. there are some uses of APPROVAL_TESTS_NO_DISCARD that don't include Macros.h. If we start by compiling all the .cpp files that only contain .h files, we will quickly detect any missing #include lines - which will then speed up the next step of moving the implementations.

(an alternative would be to get https://include-what-you-use.org set up, but I think the overheads of setting that up are too great for our current need...)

[claremacrae]

Another nice advantage of creating .cpp files in ApprovalTests/ is that people working in VisualStudio and linking against our source code will see all the files in ApprovalTests/ - which they don't, currently.

Here are the diffs from when I did a practice run of this change a few months ago...

https://github.com/approvals/ApprovalTests.cpp/compare/4cd2a8eb3d36844b8d096e669fee726181a98f7f...implementations_to_cpp

It will be worth reviewing the list of files changed, as I spotted a few things like documentation...

It's interesting how many fewer headers are needed in .h files, giving a further speed-up in build times, if we do this...

[claremacrae]

Another benefit of separating out the implementation is that we will no longer need to do the dance with initialising class-static variables like this:

https://github.com/approvals/ApprovalTests.cpp/blob/d3a4d33d994c28ec715c7cddb130ed1374dcbbdd/doc/CodingPatterns.md

As it will be easy to define the static variable in the .cpp file...

[claremacrae]

We've started work on this, on the separate_implementations branch.

[claremacrae]

Steps to remember:

• [x] integrations/
• [x] reporters/DiffPrograms.h
• [x] simulated single header - it's giving duplicate symbol errors in the two GitHub Actions unity builds
• [x] Try to remove some unused #includes
• [x] Make release script include .cpp files in actual single header only, but not the simulated one
• [x] rework release script so that the version header always appears near the top of the simulated single-header file
• [ ] Review expensive headers in *.h - so they are not compiled in to test files other than main()
  • [x] <iostream>
  • [ ] <map>
  • [ ] <fstream>
  • [ ] <sstream>
• [ ] Review the Read the Docs API docs - in case they need to change for the creation of .cpp files

[claremacrae]

Times for clean development builds on my Mac

• clean build of all of ApprovalTests.cpp
• in cmake-build-debug-clang-mp-90 - which supports Boost.UT
• ccache turned off
• with clang time-trace turned off
• run inside CLion's terminal window
• with CLion set to power-saving mode, to stop its inspections kicking on

Running the command:

make clean && time make

Old - 653e7eff83ec569b001837eb7b7df061a59876e5 - v.10.1.0 release

• make 145.39s user 7.67s system 99% cpu 2:33.77 total
• make 147.96s user 7.78s system 99% cpu 2:36.56 total

New - 50f452b7effb1877cbf2731512b8d9adcefb5a1c:

• make 103.33s user 8.59s system 98% cpu 1:53.24 total
• make 105.27s user 8.82s system 98% cpu 1:55.57 total

So that's about a 30% reduction in time for a full development build...

Development Process

But when we are adding new features, we will usually now only be changing a pair of .cpp files - one for the implementation and one for the test...

Which will be a massive improvement over the previous case, where usually most of our test .cpp files were updated every time we made an edit.

[claremacrae]

This will be released in 10.1.1