Skip to contentSkip to conversation

Fix the speed of the CI (Visual Studio) tests

PATCH Superseded Johannes Schindelin via GitGitGadget 4 participants 9 messages Oct 20, 2019 – Oct 24, 2019

lore
Share & tools
lore
#0 lore raw

cover Fix the speed of the CI (Visual Studio) tests

I made a mistake when converting the make/prove-based test job to a test-tool run-command testsuite one: I lost the parallelization, resulting in way slower CI runs.

Also, I forgot to build with DEVELOPER=1, i.e. with stricter compile flags.

This pair of patches fixes both issues.

Johannes Schindelin (2):
ci(visual-studio): use strict compile flags, and optimization ci(visual-studio): actually run the tests in parallel

 azure-pipelines.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

base-commit: d966095db01190a2196e31195ea6fa0c722aa732
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-410%2Fdscho%2Faccelerate-ci-vs-test-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-410/dscho/accelerate-ci-vs-test-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/410

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/2 ci(visual-studio): use strict compile flags, and optimization

From: Johannes Schindelin <[email protected]>

To make full use of the work that went into the Visual Studio build & test jobs in our CI/PR builds, let's turn on strict compiler flags. This will give us the benefit of Visual C's compiler warnings (which, at times, seem to catch things that GCC does not catch, and vice versa).

While at it, also turn on optimization; It does not make sense to produce binaries with debug information, and we can use any ounce of speed that we get (because the test suite is particularly slow on Windows, thanks to the need to run inside a Unix shell, which requires us to use the POSIX emulation layer provided by MSYS2).

---
 azure-pipelines.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
azure-pipelines.yml +1 -1
index 9f099b9529..457c6fee31 100644
@@ -157,7 +157,7 @@ jobs:
     displayName: 'Download git-sdk-64-minimal'
   - powershell: |
       & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
-        make vcxproj
+        make NDEBUG=1 DEVELOPER=1 vcxproj
       "@
       if (!$?) { exit(1) }
     displayName: Generate Visual Studio Solution
Signature · 1 line
gitgitgadget
#2 lore raw

PATCH 2/2 ci(visual-studio): actually run the tests in parallel

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Originally, the CI/PR builds that build and test using Visual Studio were implemented imitating linux-clang, i.e. still using the Makefile-based build infrastructure.

Later (but still before the patches made their way into git.git's master), however, this was changed to generate Visual Studio project files and build the binaries using MSBuild, as this reflects more accurately how Visual Studio users would want to build Git (internally, Visual Studio uses MSBuild, or at least something very similar).

During that transition, we needed to implement a new way to run the test suite in parallel, as Visual Studio users typically will only have a Git Bash available (which does not ship with make nore with support for prove): we simply implemented a new test helper to run the test suite.

This helper even knows how to run the tests in parallel, but due to a mistake on this developer's part, it was never turned on in the CI/PR builds. This results in 2x-3x longer run times of the test phase.

Let's use the --jobs=10 option to fix this.

---
 azure-pipelines.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
azure-pipelines.yml +1 -1
index 457c6fee31..af2a5ea484 100644
@@ -255,7 +255,7 @@ jobs:
 
         cd t &&
         PATH=\"`$PWD/helper:`$PATH\" &&
-        test-tool.exe run-command testsuite -V -x --write-junit-xml \
+        test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \
                 `$(test-tool.exe path-utils slice-tests \
                         `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)
       "@
Signature · 1 line
gitgitgadget
#3 lore raw

Re: [PATCH 2/2] ci(visual-studio): actually run the tests in parallel

Quoted 4 lines · Johannes Schindelin via GitGitGadget

[...]
During that transition, we needed to implement a new way to run the test suite in parallel, as Visual Studio users typically will only have a Git Bash available (which does not ship with make nore with support for

s/nore/nor/

Quoted 9 lines

prove): we simply implemented a new test helper to run the test suite.

This helper even knows how to run the tests in parallel, but due to a mistake on this developer's part, it was never turned on in the CI/PR builds. This results in 2x-3x longer run times of the test phase.

Let's use the --jobs=10 option to fix this.

#4 lore raw

v2 cover Fix the speed of the CI (Visual Studio) tests

In reply to Johannes Schindelin via GitGitGadget

I made a mistake when converting the make/prove-based test job to a test-tool run-command testsuite one: I lost the parallelization, resulting in way slower CI runs.

Also, I forgot to build with DEVELOPER=1, i.e. with stricter compile flags.

This pair of patches fixes both issues.

Changes since v1:

* Fixed typo "nore" -> "nor" in the commit message.

Johannes Schindelin (2):
ci(visual-studio): use strict compile flags, and optimization ci(visual-studio): actually run the tests in parallel

 azure-pipelines.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

base-commit: d966095db01190a2196e31195ea6fa0c722aa732
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-410%2Fdscho%2Faccelerate-ci-vs-test-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-410/dscho/accelerate-ci-vs-test-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/410

Range-diff vs v1:

 1:  6fba3b1c76 = 1:  6fba3b1c76 ci(visual-studio): use strict compile flags, and optimization
 2:  e3343d1740 ! 2:  07749ab720 ci(visual-studio): actually run the tests in parallel

@@ -14,7 +14,7 @@

During that transition, we needed to implement a new way to run the test suite in parallel, as Visual Studio users typically will only have a Git
- Bash available (which does not ship with make nore with support for
+ Bash available (which does not ship with make nor with support for
prove): we simply implemented a new test helper to run the test suite.

This helper even knows how to run the tests in parallel, but due to a

Signature · 1 line
gitgitgadget
#5 lore raw

PATCH v2 1/2 ci(visual-studio): use strict compile flags, and optimization

From: Johannes Schindelin <[email protected]>

To make full use of the work that went into the Visual Studio build & test jobs in our CI/PR builds, let's turn on strict compiler flags. This will give us the benefit of Visual C's compiler warnings (which, at times, seem to catch things that GCC does not catch, and vice versa).

While at it, also turn on optimization; It does not make sense to produce binaries with debug information, and we can use any ounce of speed that we get (because the test suite is particularly slow on Windows, thanks to the need to run inside a Unix shell, which requires us to use the POSIX emulation layer provided by MSYS2).

---
 azure-pipelines.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
azure-pipelines.yml +1 -1
index 9f099b9529..457c6fee31 100644
@@ -157,7 +157,7 @@ jobs:
     displayName: 'Download git-sdk-64-minimal'
   - powershell: |
       & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
-        make vcxproj
+        make NDEBUG=1 DEVELOPER=1 vcxproj
       "@
       if (!$?) { exit(1) }
     displayName: Generate Visual Studio Solution
Signature · 1 line
gitgitgadget
#6 lore raw

PATCH v2 2/2 ci(visual-studio): actually run the tests in parallel

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Originally, the CI/PR builds that build and test using Visual Studio were implemented imitating linux-clang, i.e. still using the Makefile-based build infrastructure.

Later (but still before the patches made their way into git.git's master), however, this was changed to generate Visual Studio project files and build the binaries using MSBuild, as this reflects more accurately how Visual Studio users would want to build Git (internally, Visual Studio uses MSBuild, or at least something very similar).

During that transition, we needed to implement a new way to run the test suite in parallel, as Visual Studio users typically will only have a Git Bash available (which does not ship with make nor with support for prove): we simply implemented a new test helper to run the test suite.

This helper even knows how to run the tests in parallel, but due to a mistake on this developer's part, it was never turned on in the CI/PR builds. This results in 2x-3x longer run times of the test phase.

Let's use the --jobs=10 option to fix this.

---
 azure-pipelines.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
azure-pipelines.yml +1 -1
index 457c6fee31..af2a5ea484 100644
@@ -255,7 +255,7 @@ jobs:
 
         cd t &&
         PATH=\"`$PWD/helper:`$PATH\" &&
-        test-tool.exe run-command testsuite -V -x --write-junit-xml \
+        test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \
                 `$(test-tool.exe path-utils slice-tests \
                         `$SYSTEM_JOBPOSITIONINPHASE `$SYSTEM_TOTALJOBSINPHASE t[0-9]*.sh)
       "@
Signature · 1 line
gitgitgadget
#7 lore raw
In reply to Johannes Schindelin via GitGitGadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 10 lines · writes:

Changes since v1:

* Fixed typo "nore" -> "nor" in the commit message.

Johannes Schindelin (2):
ci(visual-studio): use strict compile flags, and optimization ci(visual-studio): actually run the tests in parallel

 azure-pipelines.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Thanks.

I will take the change to 'master' directly, as cooking in 'next' won't give it any extra exposure when the topic touches only this file and the patch comes from those who do exercise azure CI build before sending it out to the list ;-)

#8 lore raw

Hi Junio,

Quoted 14 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 10 lines · writes:

Changes since v1:

* Fixed typo "nore" -> "nor" in the commit message.

Johannes Schindelin (2):
ci(visual-studio): use strict compile flags, and optimization ci(visual-studio): actually run the tests in parallel

 azure-pipelines.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Quoted 6 lines

Thanks.

I will take the change to 'master' directly, as cooking in 'next' won't give it any extra exposure when the topic touches only this file and the patch comes from those who do exercise azure CI build before sending it out to the list ;-)

Thank you very much! It is indeed a lot faster now.

Ciao,
Dscho