Merge lp:~michihenning/storage-framework/faster-header-tests into lp:storage-framework/devel

[Michi Henning]

Don't use temporary file names for header test. This hugely speeds up the header compilation
tests if ccache is enabled.

Don't use temporary file names for header test. This hugely speeds up the header compilation
tests if ccache is enabled.

[unity-api-1-bot (unity-api-1-bot)]

PASSED: Continuous integration, rev:94
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/206/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1110
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1117
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/908
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/908/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/908
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/908/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/908
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/908/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/908
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/908/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/908
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/908/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/908
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/908/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/206/rebuild

review: Approve (continuous-integration)

[James Henstridge (jamesh)]

So when using ccache, this might do less work in the long run, but will do more work in the short term since you're not just checking the syntax.

As far as stable paths go, rather than generating a simple .cpp file that includes the header, couldn't you just try to compile the header directly?

review: Needs Information

[Michi Henning (michihenning)]

Removing -fsyntax-only means that no output is generated, and ccache won't cache anything, so everything gets re-compiled every time. It turns out that the code generation part of this costs essentially nothing. All the time is spent in parsing and semantic checks, and turning off -fsyntax-only doesn't save anything you would notice.

I guess we could change the script to compile the headers directly, yes.

[lp:~michihenning/storage-framework/faster-header-tests]

95. By Michi Henning on 2016-11-21

Add test for double-inclusion of headers.

[Michi Henning (michihenning)]

I had a look at compiling the headers directly, but that is a lot more complex because the location of the header depends on the include path. It's way easier to just generate the source file and compile it like any other source file in the tree.

I made one change to the test script: it now #includes every header twice, to make sure that double-inclusion doesn't cause any problems.

I also tinkered with -fsyntax-only. It makes no perceptible difference in compilation speed. With the devel branch, it takes 29 seconds with and without this option.

Interestingly, with this branch, it only takes 25 seconds to run the header test with a cold ccache. I'm not sure where the 4-second difference comes from.

Once the cache is hot, it takes 1 second.

So, this is a big win, even without -fsyntax and without a warm ccache, and it is faster despite the double-inclusion of each header.

[unity-api-1-bot (unity-api-1-bot)]

PASSED: Continuous integration, rev:95
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/208/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1127
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1134
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/925
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/925/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/925
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/925/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/925
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/925/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/925
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/925/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/925
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/925/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/925
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/925/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/208/rebuild

review: Approve (continuous-integration)

[James Henstridge (jamesh)]

Looks good.

review: Approve

=== modified file 'tests/headers/CMakeLists.txt'
--- tests/headers/CMakeLists.txt	2016-09-08 08:33:59 +0000
+++ tests/headers/CMakeLists.txt	2016-11-21 01:32:18 +0000
@@ -41,14 +41,14 @@
     add_test(stand-alone-${location}-headers
         ${CMAKE_CURRENT_SOURCE_DIR}/compile_headers.py
             ${extra_defines}
-            ${public_inc_dir} ${CMAKE_CXX_COMPILER} "${CMAKE_CXX_COMPILER_ARG1} -fPIC -fsyntax-only -I${root_inc_dir} -I${public_inc_dir} ${other_inc_dirs} ${CMAKE_CXX_FLAGS} ${extra_inc_dirs}")
+            ${public_inc_dir} ${CMAKE_CXX_COMPILER} "${CMAKE_CXX_COMPILER_ARG1} -fPIC -I${root_inc_dir} -I${public_inc_dir} ${other_inc_dirs} ${CMAKE_CXX_FLAGS} ${extra_inc_dirs}")
 
     # Test that each internal header compiles stand-alone.
     if (IS_DIRECTORY ${internal_inc_dir})
         add_test(stand-alone-${location}-internal-headers
             ${CMAKE_CURRENT_SOURCE_DIR}/compile_headers.py
                 ${extra_defines}
-                ${internal_inc_dir} ${CMAKE_CXX_COMPILER} "${CMAKE_CXX_COMPILER_ARG1} -fPIC -fsyntax-only -I${root_inc_dir} -I${internal_inc_dir} ${other_inc_dirs} ${CMAKE_CXX_FLAGS} ${extra_inc_dirs}")
+                ${internal_inc_dir} ${CMAKE_CXX_COMPILER} "${CMAKE_CXX_COMPILER_ARG1} -fPIC -I${root_inc_dir} -I${internal_inc_dir} ${other_inc_dirs} ${CMAKE_CXX_FLAGS} ${extra_inc_dirs}")
     endif()
 
     if (NOT ${public_inc_dir} MATCHES "/internal/")

=== modified file 'tests/headers/compile_headers.py'
--- tests/headers/compile_headers.py	2016-08-22 04:35:33 +0000
+++ tests/headers/compile_headers.py	2016-11-21 01:32:18 +0000
@@ -52,7 +52,6 @@
 import shlex
 import subprocess
 import sys
-import tempfile
 import concurrent.futures, multiprocessing
 
 #
@@ -74,16 +73,20 @@
 #
 def run_compiler(hdr, compiler, copts, define, verbose, hdr_dir):
     try:
-        src = tempfile.NamedTemporaryFile(suffix='.cpp', dir='.')
+        compile_dir = "./.header_tests"
+        os.makedirs(compile_dir, exist_ok=True)
+        src_name = os.path.join(compile_dir, hdr) + ".cpp"
+
+        if not os.path.exists(src_name):
+            src_fd = os.open(src_name, os.O_WRONLY | os.O_CREAT)
+            src = os.fdopen(src_fd, 'w')
+            src.write("#include <" + hdr + ">" + "\n")
+            src.write("#include <" + hdr + ">" + "\n")  # To test that double-inclusion is safe
 
         # Add any extra defines to the command line.
         for flag in define:
             copts = "-D" + flag + " " + copts
 
-        src.write(bytes("#include <" + hdr + ">" + "\n", 'UTF-8'))
-        src.flush()                                                 # Need this to make the file visible
-        src_name = os.path.join('.', src.name)
-
         if verbose:
             print(compiler + " -c " + src_name + " " + copts)
 
@@ -91,13 +94,13 @@
         if status != 0:
             message("cannot compile \"" + hdr + "\"") # Yes, write to stdout because this is expected output
 
-        obj = os.path.splitext(src_name)[0] + ".o"
+        obj = hdr + ".o"
         try:
             os.unlink(obj)
         except:
             pass
 
-        gcov = os.path.splitext(src_name)[0] + ".gcno"
+        gcov = hdr + ".gcno"
         try:
             os.unlink(gcov)
         except: