Disable QTC inside the library by default (fixes #714)

This results in measurable performance improvements to packaged binary libqpdf distributions. QTC remains available for library users and is still selectively enabled in CI.

Disable QTC inside the library by default (fixes #714)
This results in measurable performance improvements to packaged binary libqpdf distributions. QTC remains available for library users and is still selectively enabled in CI.
Jay Berkenbilt
1 parent da71dc6f
Showing 9 changed files with 71 additions and 11 deletions
CMakeLists.txt
ChangeLog
TODO
build-scripts/test-sanitizers
include/qpdf/QTC.hh
libqpdf/QTC.cc
manual/installation.rst
manual/release-notes.rst
run-qtest
@@ -43,6 +43,9 @@ CMAKE_DEPENDENT_OPTION(
   GENERATE_AUTO_JOB "Automatically regenerate job files" OFF
   "NOT MAINTAINER_MODE" ON)
 CMAKE_DEPENDENT_OPTION(
+  ENABLE_QTC "Enable QTC test coverage" OFF
+  "NOT MAINTAINER_MODE" ON)
+CMAKE_DEPENDENT_OPTION(
   SHOW_FAILED_TEST_OUTPUT "Show qtest output on failure" OFF
   "NOT CI_MODE" ON)
@@ -110,8 +113,15 @@ endif()
 add_compile_definitions($<$<COMPILE_LANGUAGE:CXX>:POINTERHOLDER_TRANSITION=4>)
+if(ENABLE_QTC)
+  set(ENABLE_QTC_ARG)
+else()
+  add_compile_definitions(QPDF_DISABLE_QTC=1)
+  set(ENABLE_QTC_ARG --disable-tc)
+endif()
+
 enable_testing()
-set(RUN_QTEST perl ${qpdf_SOURCE_DIR}/run-qtest)
+set(RUN_QTEST perl ${qpdf_SOURCE_DIR}/run-qtest ${ENABLE_QTC_ARG})
 if(WIN32)
   find_program(COPY_COMMAND NAMES cp copy)
@@ -335,6 +345,7 @@ message(STATUS &quot;  build shared libraries: ${BUILD_SHARED_LIBS}&quot;)
 message(STATUS "  build static libraries: ${BUILD_STATIC_LIBS}")
 message(STATUS "  build manual: ${BUILD_DOC}")
 message(STATUS "  compiler warnings are errors: ${WERROR}")
+message(STATUS "  QTC test coverage: ${ENABLE_QTC}")
 message(STATUS "  system: ${CPACK_SYSTEM_NAME}")
 message(STATUS "")
 message(STATUS "*** Options Summary ***")
+2022-08-07  Jay Berkenbilt  <ejb@ql.org>
+
+        * Add new build configuration option ENABLE_QTC, which is off by
+        default when not running in MAINTAINER_MODE. When this is off,
+        QTC coverage calls sprinkled throughout the qpdf source code are
+        compiled out for increased performance. See "Build Options" in the
+        manual for a discussion. Fixes #714.
+
 2022-08-06  Jay Berkenbilt  <ejb@ql.org>
         * Added by m-holger: QPDF::getObject() method as a simpler form of
@@ -4,14 +4,9 @@ Next
 Before Release:
-* Improve performance around QTC
-  * Make it possible to compile it out and deal with it in the tests
-  * Make sure at least some CI test is run with coverage enabled
-  * Cache environment variables
-  * Remove coverage cases for things that are heavily exercised or are
-    in critical paths
 * Make ./performance_check usable by other people by having published
   files to use for testing.
+  * https://opensource.adobe.com/dc-acrobat-sdk-docs/standards/pdfstandards/pdf/PDF32000_2008.pdf
 * Evaluate issues tagged with `next`
 * Stay on top of https://github.com/pikepdf/pikepdf/pull/315
@@ -10,7 +10,8 @@ env CFLAGS=&quot;-fsanitize=address -fsanitize=undefined&quot; \
     CC=clang CXX=clang++ \
     cmake -S . -B build \
     -DCI_MODE=1 -DBUILD_SHARED_LIBS=0 -DCMAKE_BUILD_TYPE=Debug \
-    -DREQUIRE_CRYPTO_OPENSSL=1 -DREQUIRE_CRYPTO_GNUTLS=1
+    -DREQUIRE_CRYPTO_OPENSSL=1 -DREQUIRE_CRYPTO_GNUTLS=1 \
+    -DENABLE_QTC=1
 cmake --build build -j$(nproc) -- -k
 cd build
 # libtests automatically runs with all crypto providers.
@@ -24,10 +24,24 @@
 #include <qpdf/DLL.h>
+// Defining QPDF_DISABLE_QTC will effectively compile out any QTC::TC
+// calls in any code that includes this file, but QTC will still be
+// built into the library. That way, it is possible to build and
+// package qpdf with QPDF_DISABLE_QTC while still making QTC::TC
+// available to end users.
+
 namespace QTC
 {
     QPDF_DLL
-    void TC(char const* const scope, char const* const ccase, int n = 0);
+    void TC_real(char const* const scope, char const* const ccase, int n = 0);
+
+    inline void
+    TC(char const* const scope, char const* const ccase, int n = 0)
+    {
+#ifndef QPDF_DISABLE_QTC
+        TC_real(scope, ccase, n);
+#endif // QPDF_DISABLE_QTC
+    }
 }; // namespace QTC
 #endif // QTC_HH
@@ -13,7 +13,7 @@ tc_active(char const* const scope)
 }
 void
-QTC::TC(char const* const scope, char const* const ccase, int n)
+QTC::TC_real(char const* const scope, char const* const ccase, int n)
 {
     static std::map<std::string, bool> active;
     auto is_active = active.find(scope);
@@ -257,6 +257,16 @@ CHECK_SIZES
   that ensures an exact match between classes in ``sizes.cc`` and
   classes in the library's public API. This option requires Python 3.
+ENABLE_QTC
+  This is off by default, except in maintainer mode. When off,
+  ``QTC::TC`` calls are compiled out by having ``QTC::TC`` be an empty
+  inline function. The underlying ``QTC::TC`` remains in the library,
+  so it is possible to build and package the qpdf library with
+  ``ENABLE_QTC`` turned off while still allowing developer code to use
+  ``QTC::TC`` if desired. If you are modifying qpdf code, it's a good
+  idea to have this on for more robust automated testing. Otherwise,
+  there's no reason to have it on.
+
 GENERATE_AUTO_JOB
   Some qpdf source files are automatically generated from
   :file:`job.yml` and the CLI documentation. If you are adding new
@@ -297,6 +307,8 @@ MAINTAINER_MODE
   - ``CHECK_SIZES``
+  - ``ENABLE_QTC``
+
   - ``GENERATE_AUTO_JOB``
   - ``WERROR``
@@ -7,6 +7,12 @@ For a detailed list of changes, please see the file
 :file:`ChangeLog` in the source distribution.
 11.0.0
+  - Performance improvements
+
+    - Many performance enhancements have been added. In developer
+      performance benchmarks, gains on the order of 20% have been
+      observed.
+
   - Replacement of ``PointerHolder`` with ``std::shared_ptr``
     - The qpdf-specific ``PointerHolder`` smart pointer implementation
@@ -231,6 +237,14 @@ For a detailed list of changes, please see the file
     - The qpdf source code is now formatted automatically with
       ``clang-format``. See :ref:`code-formatting` for information.
+    - Test coverage with ``QTC`` is enabled during development but
+      compiled out of distributed qpdf binaries by default. This
+      results in a significant performance improvement, especially on
+      Windows. ``QTC::TC`` is still available in the library and is
+      still usable by end user code even though calls to it made
+      internally by the library are turned off. Internally, there is
+      some additional caching to reduce the overhead of repeatedly
+      reading environment variables at runtime.
 10.6.3: March 8, 2022
   - Announcement of upcoming change:
@@ -13,6 +13,7 @@ my $code = undef;
 my @bin = ();
 my $color = undef;
 my $show_on_failure = 0;
+my $disable_tc = 0;
 my @tc = ();
 if ($^O =~ m/^MSWin32|msys$/)
@@ -51,6 +52,10 @@ while (@ARGV)
         usage() unless @ARGV;
         $show_on_failure = cmake_bool(shift(@ARGV));
     }
+    elsif ($arg eq '--disable-tc')
+    {
+        $disable_tc = 1;
+    }
     elsif ($arg eq '--tc')
     {
         usage() unless @ARGV;
@@ -94,7 +99,7 @@ push(@cmd,
      "-datadir", "$code/qtest",
      "-junit-suffix", basename($code));
-if (scalar(@tc))
+if (scalar(@tc) && (! $disable_tc))
 {
     my @tc_srcs = map {
         File::Spec->abs2rel(abs_path($_))