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.

9 files changed, 71 insertions, 11 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3e50f0b5..2581fd7f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -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 "  build shared libraries: ${BUILD_SHARED_LIBS}")
 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 ***")
diff --git a/ChangeLog b/ChangeLog
index bc479f17..b861ddda 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+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
diff --git a/TODO b/TODO
index fc007c98..1b452805 100644
--- a/TODO
+++ b/TODO
@@ -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
 
diff --git a/build-scripts/test-sanitizers b/build-scripts/test-sanitizers
index c3c314f9..75ac8af0 100755
--- a/build-scripts/test-sanitizers
+++ b/build-scripts/test-sanitizers
@@ -10,7 +10,8 @@ env CFLAGS="-fsanitize=address -fsanitize=undefined" \
     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.
diff --git a/include/qpdf/QTC.hh b/include/qpdf/QTC.hh
index 1fa55901..70115981 100644
--- a/include/qpdf/QTC.hh
+++ b/include/qpdf/QTC.hh
@@ -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
diff --git a/libqpdf/QTC.cc b/libqpdf/QTC.cc
index 21d240ba..1ca79c05 100644
--- a/libqpdf/QTC.cc
+++ b/libqpdf/QTC.cc
@@ -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);
diff --git a/manual/installation.rst b/manual/installation.rst
index e02380ee..08c49765 100644
--- a/manual/installation.rst
+++ b/manual/installation.rst
@@ -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``
diff --git a/manual/release-notes.rst b/manual/release-notes.rst
index ebbfd4f5..ab2c1d8e 100644
--- a/manual/release-notes.rst
+++ b/manual/release-notes.rst
@@ -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:
diff --git a/run-qtest b/run-qtest
index 1160589d..481052e9 100755
--- a/run-qtest
+++ b/run-qtest
@@ -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($_))