Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernised CMakePresets.json to make it more useful for developers #1723

Conversation

auto-differentiation-dev
Copy link
Contributor

@auto-differentiation-dev auto-differentiation-dev commented Jul 4, 2023

This PR modernises the CMakePresets.json to make it more useful and less repetitive for developers (not just for CI/CD).

Configure Presets

It defines a set of hidden, base configurations, and uses inherit to combine them into user-accessible configure presets for each Windows/Linux with MSVC/Clang/GCC. This makes the dropdown with the Visual Studio "Open Folder" feature look like this for a Windows target:

image

When selecting a remote or WSL Linux target machine, the drop-down looks like this:

image

For Linux, both Unix Makefiles (default) and Ninja generator presets are there.

The same configuration presets are of course also available on the command-line with cmake --preset xxxx.

The build directory is set to ${sourceDir}/build/${presetName}, so all presets can live side-by-side. The corresponding CI/CD workflow jobs have been adjusted to use the new path.

Build Presets

The build presets have been removed - they didn't add any additional configuration to what's already covered by the configure presets and were quite repetitive. The Visual Studio IDE works just fine without, and for building on the command-line, users can just do:

cd build/name-of-preset
cmake --build . 

Developer-specific Adjustments

Often developers want to add their own configurations or settings in their own presets. This can be done if they add a CMakeUserPresets.json file to the QuantLib root and add additional configurations. With the new hidden base configurations in this PR, it makes it easier to inherit combinations that users need and add new settings, for example:

CMakeUserPresets.json

{
  "version": 3,
  "configurePresets": [
    {
      "name": "my-preset-debug",
      "inherits": [
        "windows-clang-base",
        "debug"
      ],
      "cacheVariables": {
        "QL_USE_STD_CLASSES": "ON"
      }
    }
  ]
}

Build Speedup Visual Studio

The current CMake-based Visual Studio MSBuild generators (multi-config), for example when using the CMake GUI to generate projects and solutions, show very poor build performance. It's barely usable as it is. This PR makes the presets much more usable out of the box for Windows users.

To compare - on a box with 18 cores (36 hyperthreads), with CMake-generated VS2022 project files, fresh release build (Xeon W-2295 CPU):

MSBuild project files: 24min 36sec

Ninja : 3min 5sec (nearly 8x faster)

Documentation Changes

Once this PR is accepted, we will prepare a change to the QuantLib build doc on the website which will explain better how to use the presets and how to setup user presets.

@sweemer
Copy link
Contributor

sweemer commented Jul 4, 2023

less repetitive

FYI, Luigi previously stated a preference for repetition over inheritance in #1209 (comment)

@coveralls
Copy link

coveralls commented Jul 4, 2023

Coverage Status

coverage: 71.881%. remained the same when pulling c04310f on auto-differentiation-dev:feature/modernize-cmake-presets into 0476dae on lballabio:master.

@sweemer
Copy link
Contributor

sweemer commented Jul 4, 2023

I might be wrong but I think that the configuration option in the multi-config build presets is required for generator expressions to work properly. Even though QuantLib's CMakeLists.txt file doesn't use any generator expressions now, it might in the future, so we should double check that this works as intended before removing the multi-config build presets.

@lballabio
Copy link
Owner

FYI, Luigi previously stated a preference for repetition over inheritance in #1209 (comment)

Qualified with a "maybe" 😄

I don't have enough experience with cmake, so if you don't mind I'll let you people work out for a while what is common practice in cmake world and what is preference. Let's regroup after I release 1.31.

@auto-differentiation-dev
Copy link
Contributor Author

I might be wrong but I think that the configuration option in the multi-config build presets is required for generator expressions to work properly.

Note that none of the proposed presets are multi-config. The Windows ones are all using the Ninja generator (which is a good bit faster and works well with the Visual Studio built-in CMake support, and also with VSCode). And Makefiles / Ninja on Linux are also single-config. Generator expressions work fine in this case. Hence with these presets, we wouldn't need the build presets.

@auto-differentiation-dev
Copy link
Contributor Author

auto-differentiation-dev commented Jul 4, 2023

Note that the current CMake-based Visual Studio MSBuild generators (multi-config), for example when using the CMake GUI to generate projects and solutions, show very poor build performance. Only a little more than 1 core is used for the build. It's barely usable as it is. On a multi-core machine, the Ninja generator with the associated build parallelism is a lot faster, as all cores are 100% utilised for the full duration of the build.

To compare - on a box with 18 cores (36 hyperthreads), with CMake-generated VS2022 project files, fresh release build (Xeon W-2295 CPU):

  • MSBuild project files: 24min 36sec
  • Ninja : 3min 5sec (nearly 8x faster)

We believe using Ninja only in the shipped presets in Windows makes this a lot more usable out of the box for Windows users. If developers need other settings, they can always use CMakeUserPresets.json for their own settings.

sudo cmake --build --preset linux-ci-build-with-nonstandard-options --target install
cd build/linux-ci-build-with-nonstandard-options
cmake --build . --verbose
cmake --build . --target install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sudo was there because /usr/local/lib is not writable by the CI user on this VM. You'll need to add it back to get the CI passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - that's done.

@sweemer
Copy link
Contributor

sweemer commented Jul 4, 2023

Note that none of the proposed presets are multi-config. The Windows ones are all using the Ninja generator (which is a good bit faster and works well with the Visual Studio built-in CMake support, and also with VSCode).

You're right, I forgot that you switched the Windows generator to Ninja in #1429. Probably we should have removed the configuration option from the Windows build presets at that time, but it's not a big deal.

And Makefiles / Ninja on Linux are also single-config. Generator expressions work fine in this case. Hence with these presets, we wouldn't need the build presets.

What if somebody adds a configuration preset for the multi-config XCode generator one day? Or would you recommend using the Ninja generator on macOS too?

@auto-differentiation-dev
Copy link
Contributor Author

What if somebody adds a configuration preset for the multi-config XCode generator one day? Or would you recommend using the Ninja generator on macOS too?

Unfortunately we didn't try that - macOS presets should probably be added as well. Very likely the Linux/Clang ones will work just fine on macOS as well though, using Ninja, as we can see from this CI build. Perhaps someone actively using macOS can test and contribute macOS presets if required separately?

For any configurations that aren't covered by the provided presets, there's always CMakeUserPresets.json. These can contain build presets too, that's not an issue. Or they can call CMake without using presets. Not sure how XCode's multi-configuration projects work and how well they perform compared to Ninja, but in our experience Ninja is faster than anything else we've tried.

We can't cover all combinations (Eclipse, CLion, etc.), but the purpose here is to cover usable common presets that work straight out of the box.

@sweemer
Copy link
Contributor

sweemer commented Jul 5, 2023

Sounds good. I don't have any more comments at this time.

@auto-differentiation-dev
Copy link
Contributor Author

@lballabio - It looks like we've reached a consensus on this.

The failing check for doc generation seems to have nothing to do with this change - perhaps it just needs to be re-run?

@lballabio
Copy link
Owner

Ok, thanks. I'm rerunning the failed check.

@lballabio lballabio added this to the Release 1.32 milestone Jul 20, 2023
@lballabio lballabio merged commit 537e4cf into lballabio:master Jul 20, 2023
@auto-differentiation-dev auto-differentiation-dev deleted the feature/modernize-cmake-presets branch March 22, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants