benchmark for SubscribeAsync (#774)

* benchmark for SubscribeAsync * properly verbose_make_output default OFF * WIN32 build of bench * WIN32 build of bench, +1 * WIN32 build of bench, +2 * PR feedback: removed irrelevant CMake options and comments * bigger matrix, occasional flush, shorter delay, diffstat * experimental: more flush * refactor test list * moved bench to test refactored bench Added BenchSubscribeAsync_Inject Added BenchSubscribeAsync_InjectSlow * PR feedback: move stopServer before CloseAndWait * Fix Win32 ctest and various warnings * comment
nats-io · Jul 27, 2024 · d8f52d9 · d8f52d9
1 parent bd700a9
commit d8f52d9
Show file tree

Hide file tree

Showing 30 changed files with 1,754 additions and 1,732 deletions.
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
@@ -48,7 +48,11 @@ on:
         default: "OFF"
       verbose_make_output:
         type: string
-        default: "ON"
+        default: "OFF"
+      benchmark:
+        type: string
+        default: "OFF"
+
     secrets:
       CODECOV_TOKEN:
         description: "Codecov repo token"
@@ -129,15 +133,6 @@ jobs:
           cmake .. ${{ steps.cmake-flags.outputs.flags }}
           make rebuild_cache && make
 
-      - name: "Rebuild the list of tests to match the compile flags"
-        working-directory: ./build
-        run: |
-          ./bin/testsuite
-          if [[ $(diff list.txt ../test/list.txt; echo $?) != 0 ]]; then
-            mv list.txt ../test/list.txt
-            make rebuild_cache
-          fi
-
       # testing
 
       - name: "Download nats-server version ${{ inputs.server_version }}"
@@ -168,12 +163,13 @@ jobs:
           fi
 
       - name: "Test"
+        if: inputs.benchmark == 'OFF'
         working-directory: ./build
         run: |
           export PATH=../deps/nats-server:../deps/nats-streaming-server:$PATH
           export NATS_TEST_SERVER_VERSION="$(nats-server -v)"
           flags=""
-          ctest --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }}
+          ctest -L 'test' --timeout 60 --output-on-failure --repeat-until-fail ${{ inputs.repeat }}
 
       - name: Upload coverage reports to Codecov
         # PRs from external contributors fail: https://github.com/codecov/feedback/issues/301
@@ -183,4 +179,42 @@ jobs:
         with:
           fail_ci_if_error: true
           token: ${{ secrets.CODECOV_TOKEN }}
-          # verbose: true
+          # verbose: true
+
+      - name: "Download benchmark results from ${{ github.event.pull_request.base.ref }}"
+        if: inputs.benchmark == 'ON' && github.event.pull_request.base.ref
+        uses: actions/download-artifact@v2
+        with:
+          name: benchmark_results_${{ github.event.pull_request.base.ref }}
+          path: ./build/prev_bench.log
+        continue-on-error: true
+
+      - name: "Benchmark"
+        if: inputs.benchmark == 'ON'
+        working-directory: ./build
+        run: |
+          export PATH=../deps/nats-server:../deps/nats-streaming-server:$PATH
+          export NATS_TEST_SERVER_VERSION="$(nats-server -v)"
+          flags=""
+          ctest -L 'bench' --timeout 600 -VV | tee bench.log
+          #
+          # ...coming: compare to base branch
+
+      - name: "Upload benchmark result for PR ${{ github.event.pull_request.head.ref }}"
+        if: inputs.benchmark == 'ON' && github.event.pull_request.head.ref
+        uses: actions/upload-artifact@v4
+        with:
+          name: benchmark_results_${{ github.event.pull_request.head.ref }}
+          path: ./build/bench.log
+
+      - name: Extract branch name
+        if: inputs.benchmark == 'ON' && !github.event.pull_request.head.ref
+        id: extract_branch
+        run: echo "BRANCH_NAME=${GITHUB_REF#refs/heads/}" >> $GITHUB_ENV
+
+      - name: "Upload benchmark result for branch ${{ env.BRANCH_NAME }}"
+        if: inputs.benchmark == 'ON' && !github.event.pull_request.head.ref
+        uses: actions/upload-artifact@v4
+        with:
+          name: benchmark_results_${{ env.BRANCH_NAME }}
+          path: ./build/bench.log
diff --git a/.github/workflows/on-pr-debug.yml b/.github/workflows/on-pr-debug.yml
@@ -1,4 +1,4 @@
-name: "Debug"
+name: "PR"
 on:
   pull_request:
 
@@ -29,7 +29,7 @@ jobs:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
   coverage-pooled:
-    name: "Coverage"
+    name: "Coverage (pooled delivery)"
     uses: ./.github/workflows/build-test.yml
     with:
       coverage: ON
@@ -74,6 +74,14 @@ jobs:
       server_version: main
       lib_write_deadline: ON
 
+  bench:
+    name: "Benchmark"
+    uses: ./.github/workflows/build-test.yml
+    with:
+      server_version: main
+      benchmark: ON
+      type: Release
+
   Windows:
     name: "Windows"
     runs-on: windows-latest
@@ -92,10 +100,38 @@ jobs:
         env:
           VCPKG_BINARY_SOURCES: "clear;x-gha,readwrite"
         run: |
-          cmake -B build -S . -DCMAKE_C_FLAGS=/W4 -DNATS_BUILD_STREAMING=OFF
-          cmake --build build --config Debug
+          cmake -B build -S . -DCMAKE_BUILD_TYPE=Debug -DNATS_BUILD_STREAMING=OFF
+          cmake --build build
 
       - name: Test
+        shell: bash
         run: |
           cd build
-          ./bin/Debug/testsuite
+          # Download latest nats-server
+          rel="latest" # TODO: parameterize
+          if [ "$rel" = "latest" ]; then
+            rel=$(curl -s https://api.github.com/repos/nats-io/nats-server/releases/latest | jq -r '.tag_name')
+          fi
+          if [ "$rel" != "${rel#v}" ] && wget https://github.com/nats-io/nats-server/releases/download/$rel/nats-server-$rel-windows-amd64.tar.gz; then
+            tar -xzf nats-server-$rel-linux-amd64.tar.gz
+            cp nats-server-$rel-windows-amd64/nats-server.exe ../deps/nats-server/nats-server.exe
+          else
+            for c in 1 2 3 4 5
+            do
+              echo "Attempt $c to download binary for main"
+              rm -f ./nats-server
+              curl -sf "https://binaries.nats.dev/nats-io/nats-server/v2@$rel" | PREFIX=. sh
+              # We are sometimes getting nats-server of size 0. Make sure we have a
+              # working nats-server by making sure we get a version number.
+              mv ./nats-server ./nats-server.exe
+              v="$(./nats-server.exe -v)"
+              if [ "$v" != "" ]; then
+                break
+              fi
+            done
+            mkdir -p ../deps/nats-server
+            mv ./nats-server.exe ../deps/nats-server/nats-server.exe
+          fi
+          export PATH=../deps/nats-server:$PATH
+          export NATS_TEST_SERVER_VERSION="$(nats-server -v)"
+          ctest -L test -C Debug --timeout 60 --output-on-failure --repeat until-pass:3
diff --git a/.github/workflows/on-push-release-extra.yml b/.github/workflows/on-push-release-extra.yml
@@ -5,7 +5,6 @@ on:
       - main
       - release_*
 
-
 permissions:
   contents: write # required by build-test to comment on coverage but not used here.
 
@@ -67,3 +66,11 @@ jobs:
       type: Debug
     secrets:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+
+  bench:
+    name: "Benchmark"
+    uses: ./.github/workflows/build-test.yml
+    with:
+      server_version: main
+      benchmark: ON
+      type: Release
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -7,6 +7,9 @@ include(FindPackageHandleStandardArgs)
 # Uncomment to have the build process verbose
 # set(CMAKE_VERBOSE_MAKEFILE TRUE)
 
+set(CMAKE_C_STANDARD 99)
+set(CMAKE_C_STANDARD_REQUIRED ON)
+
 # Set output directories for libraries and executables.
 # This is important for Windows builds to have the DLLs in the same directory as the executables.
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
@@ -202,6 +205,14 @@ elseif(WIN32)
   set(NATS_EXTRA_LIB "ws2_32")
   set(NATS_OS "_WIN32")
   set(NATS_PLATFORM_INCLUDE "win")
+
+  # Warning control.
+  add_compile_options(/W4) # Set warning level to maximum, then disable:
+  add_compile_options(/wd4100) # unreferenced formal parameter
+  add_compile_options(/wd4200) # nonstandard extension used: zero-sized array in struct/union
+  add_compile_options(/wd4130) # logical operation on address of string constant
+  add_compile_options(/wd4127) # conditional expression is constant
+
   if(sodium_USE_STATIC_LIBS)
     add_definitions(
       -DSODIUM_STATIC

diff --git a/README.md b/README.md
@@ -241,38 +241,15 @@ ctest -T memcheck -V -I 1,4
 ```
 The above command would run the tests with `valgrind` (`-T memcheck`), with verbose output (`-V`), and run the tests from 1 to 4 (`-I 1,4`).
 
-If you add a test to `test/test.c`, you need to add it into the `allTests` array. Each entry contains a name, and the test function. You can add it anywhere into this array.
-Build you changes:
+If you add a test to `test/test.c`, you need to add it into the `list_test.txt`
+file. Each entry contains just the test name, the function must be named
+identically, with a `test_` prefix. The list is in alphabetical order, but it
+does not need to be, you can add anywhere.
 
-```
-make
-[ 44%] Built target nats
-[ 88%] Built target nats_static
-[ 90%] Built target nats-publisher
-[ 92%] Built target nats-queuegroup
-[ 94%] Built target nats-replier
-[ 96%] Built target nats-requestor
-[ 98%] Built target nats-subscriber
-Scanning dependencies of target testsuite
-[100%] Building C object test/CMakeFiles/testsuite.dir/test.c.o
-Linking C executable testsuite
-[100%] Built target testsuite
-```
-
-Now regenerate the list by invoking the test suite without any argument:
-
-```
-./test/testsuite
-Number of tests: 77
-```
-
-This list the number of tests added to the file `list.txt`. Move this file to the source's test directory.
-
-```
-mv list.txt ../test/
-```
+If you are adding a benchmark, it should be added to the `list_bench.txt`. These
+tests are labeled differently (`-L 'bench'`) and executed separately on CI.
 
-Then, refresh the build:
+You need to re-run `cmake` for the changes to take effect:
 
 ```
 cmake ..

diff --git a/buildOnTravis.sh b/buildOnTravis.sh
@@ -75,7 +75,7 @@ fi
 
 export NATS_TEST_TRAVIS=yes
 echo "Using NATS server version: $NATS_TEST_SERVER_VERSION"
-ctest --timeout 60 --output-on-failure $4
+ctest -L 'test' --timeout 60 --output-on-failure $4
 res=$?
 if [ $res -ne 0 ]; then
   exit $res

diff --git a/examples/examples.h b/examples/examples.h
@@ -45,7 +45,7 @@ volatile int64_t elapsed = 0;
 bool             print   = false;
 int64_t          timeout = 10000; // 10 seconds.
 
-natsOptions      *opts   = NULL;
+natsOptions      *gOpts   = NULL;
 
 const char       *certFile = NULL;
 const char       *keyFile  = NULL;
@@ -154,7 +154,7 @@ printUsageAndExit(const char *progName, const char *usage)
                 "%s\n",
                 progName, usage);
 
-    natsOptions_Destroy(opts);
+    natsOptions_Destroy(gOpts);
     nats_Close();
 
     exit(1);
@@ -214,7 +214,7 @@ parseArgs(int argc, char **argv, const char *usage)
     bool        urlsSet = false;
     int         i;
 
-    if (natsOptions_Create(&opts) != NATS_OK)
+    if (natsOptions_Create(&gOpts) != NATS_OK)
         s = NATS_NO_MEMORY;
 
     for (i=1; (i<argc) && (s == NATS_OK); i++)
@@ -229,20 +229,20 @@ parseArgs(int argc, char **argv, const char *usage)
             if (i + 1 == argc)
                 printUsageAndExit(argv[0], usage);
 
-            s = parseUrls(argv[++i], opts);
+            s = parseUrls(argv[++i], gOpts);
             if (s == NATS_OK)
                 urlsSet = true;
         }
         else if (strcasecmp(argv[i], "-tls") == 0)
         {
-            s = natsOptions_SetSecure(opts, true);
+            s = natsOptions_SetSecure(gOpts, true);
         }
         else if (strcasecmp(argv[i], "-tlscacert") == 0)
         {
             if (i + 1 == argc)
                 printUsageAndExit(argv[0], usage);
 
-            s = natsOptions_LoadCATrustedCertificates(opts, argv[++i]);
+            s = natsOptions_LoadCATrustedCertificates(gOpts, argv[++i]);
         }
         else if (strcasecmp(argv[i], "-tlscert") == 0)
         {
@@ -263,18 +263,18 @@ parseArgs(int argc, char **argv, const char *usage)
             if (i + 1 == argc)
                 printUsageAndExit(argv[0], usage);
 
-            s = natsOptions_SetCiphers(opts, argv[++i]);
+            s = natsOptions_SetCiphers(gOpts, argv[++i]);
         }
         else if (strcasecmp(argv[i], "-tlshost") == 0)
         {
             if (i + 1 == argc)
                 printUsageAndExit(argv[0], usage);
 
-            s = natsOptions_SetExpectedHostname(opts, argv[++i]);
+            s = natsOptions_SetExpectedHostname(gOpts, argv[++i]);
         }
         else if (strcasecmp(argv[i], "-tlsskip") == 0)
         {
-            s = natsOptions_SkipServerVerification(opts, true);
+            s = natsOptions_SkipServerVerification(gOpts, true);
         }
         else if (strcasecmp(argv[i], "-sync") == 0)
         {
@@ -323,7 +323,7 @@ parseArgs(int argc, char **argv, const char *usage)
         }
         else if (strcasecmp(argv[i], "-gd") == 0)
         {
-            s = natsOptions_UseGlobalMessageDelivery(opts, true);
+            s = natsOptions_UseGlobalMessageDelivery(gOpts, true);
         }
         else if (strcasecmp(argv[i], "-c") == 0)
         {
@@ -377,14 +377,14 @@ parseArgs(int argc, char **argv, const char *usage)
             if (i + 1 == argc)
                 printUsageAndExit(argv[0], usage);
 
-            s = natsOptions_SetUserCredentialsFromFiles(opts, argv[++i], NULL);
+            s = natsOptions_SetUserCredentialsFromFiles(gOpts, argv[++i], NULL);
         }
         else if (strcasecmp(argv[i], "-wd") == 0)
         {
             if (i + 1 == argc)
                 printUsageAndExit(argv[0], usage);
 
-            s = natsOptions_SetWriteDeadline(opts, atol(argv[++i]));
+            s = natsOptions_SetWriteDeadline(gOpts, atol(argv[++i]));
         }
         else if (strcasecmp(argv[i], "-stream") == 0)
         {
@@ -410,10 +410,10 @@ parseArgs(int argc, char **argv, const char *usage)
     }
 
     if ((s == NATS_OK) && ((certFile != NULL) || (keyFile != NULL)))
-        s = natsOptions_LoadCertificatesChain(opts, certFile, keyFile);
+        s = natsOptions_LoadCertificatesChain(gOpts, certFile, keyFile);
 
     if ((s == NATS_OK) && !urlsSet)
-        s = parseUrls(NATS_DEFAULT_URL, opts);
+        s = parseUrls(NATS_DEFAULT_URL, gOpts);
 
     if (s != NATS_OK)
     {
@@ -422,12 +422,12 @@ parseArgs(int argc, char **argv, const char *usage)
 
         nats_PrintLastErrorStack(stderr);
 
-        natsOptions_Destroy(opts);
+        natsOptions_Destroy(gOpts);
         nats_Close();
         exit(1);
     }
 
-    return opts;
+    return gOpts;
 }
 
 #endif /* EXAMPLES_H_ */