From 74bc7f6bd408992801438d70d0bb24e41eb4603d Mon Sep 17 00:00:00 2001 From: Egor Mikhaylov Date: Fri, 22 Nov 2024 19:21:00 +0300 Subject: [PATCH 1/6] Improve binding build times. --- .github/workflows/build-test-macos.yml | 4 +- .github/workflows/build-test-windows.yml | 2 +- .github/workflows/pip-build.yml | 2 +- .github/workflows/prepare-images.yml | 1 + scripts/mrbind/compiler_only_flags.txt | 1 + scripts/mrbind/generate.mk | 40 ++++++++++++++++++- scripts/mrbind/install_deps_ubuntu.sh | 3 +- scripts/mrbind/install_deps_windows_msys2.bat | 5 ++- scripts/mrbind/mrbind_commit.txt | 2 +- scripts/mrbind/mrbind_flags.txt | 2 +- scripts/mrbind/mrbind_flags_for_helpers.txt | 2 +- source/MRMesh/MRObject.h | 4 +- 12 files changed, 54 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build-test-macos.yml b/.github/workflows/build-test-macos.yml index b09495b61378..f48c9e9ac955 100644 --- a/.github/workflows/build-test-macos.yml +++ b/.github/workflows/build-test-macos.yml @@ -94,15 +94,13 @@ jobs: if: ${{inputs.mrbind}} env: PATH: ${{matrix.brewpath}}/opt/make/libexec/gnubin:${{matrix.brewpath}}/opt/grep/libexec/gnubin:${{env.PATH}} - # Using 4 threads on github runners, because they have only 8G of RAM. run: | make --version make -f scripts/mrbind/generate.mk \ -B --trace \ PYTHON_PKGCONF_NAME=python-3.10-embed \ EXTRA_CFLAGS='-DMB_PB11_ALLOW_STD_EXPECTED=0 -DMR_USE_STD_EXPECTED=0' \ - MESHLIB_SHLIB_DIR=build/${{matrix.config}}/bin \ - ${{ fromJSON('["-j8", "-j4"]')[matrix.os == 'github-arm'] }} + MESHLIB_SHLIB_DIR=build/${{matrix.config}}/bin - name: Run Start-and-Exit Tests if: ${{ matrix.os == 'arm' }} diff --git a/.github/workflows/build-test-windows.yml b/.github/workflows/build-test-windows.yml index 5960b3dad572..e66696eb70fe 100644 --- a/.github/workflows/build-test-windows.yml +++ b/.github/workflows/build-test-windows.yml @@ -147,7 +147,7 @@ jobs: if: ${{inputs.mrbind}} with: msystem: clang64 - install: make gawk + install: make gawk procps-ng location: C:\msys64_meshlib_mrbind - name: Install Clang in MSYS2 diff --git a/.github/workflows/pip-build.yml b/.github/workflows/pip-build.yml index 58c83b1cc6b5..62cfb27cba93 100644 --- a/.github/workflows/pip-build.yml +++ b/.github/workflows/pip-build.yml @@ -288,7 +288,7 @@ jobs: uses: msys2/setup-msys2@v2 with: msystem: clang64 - install: make gawk + install: make gawk procps-ng location: C:\msys64_meshlib_mrbind - name: Install Clang in MSYS2 diff --git a/.github/workflows/prepare-images.yml b/.github/workflows/prepare-images.yml index d15737b6d68e..fe5c792654cb 100644 --- a/.github/workflows/prepare-images.yml +++ b/.github/workflows/prepare-images.yml @@ -46,6 +46,7 @@ jobs: - 'scripts/build_thirdparty.sh' - 'scripts/install_apt_requirements.sh' - 'scripts/install_dnf_requirements.sh' + - 'scripts/mrbind/install_deps_ubuntu.sh' - 'thirdparty/!(install.bat|vcpkg/**)' - name: Filter Windows paths diff --git a/scripts/mrbind/compiler_only_flags.txt b/scripts/mrbind/compiler_only_flags.txt index e9d42e20cb69..3b511d3dd0bc 100644 --- a/scripts/mrbind/compiler_only_flags.txt +++ b/scripts/mrbind/compiler_only_flags.txt @@ -7,3 +7,4 @@ -DMB_PB11_STRIPPED_NAMESPACES='"MR","MR.Extra"' -DMB_PB11_ENABLE_CXX_STYLE_CONTAINER_METHODS -DMB_PB11_MERGE_STL_TL_EXPECTED +-DMB_PB11_NO_REGISTER_TYPE_DEPS diff --git a/scripts/mrbind/generate.mk b/scripts/mrbind/generate.mk index 3618c8137f0f..d930749cac4a 100644 --- a/scripts/mrbind/generate.mk +++ b/scripts/mrbind/generate.mk @@ -250,12 +250,48 @@ PCH_CODEGEN_FLAGS := # The `-fpch-instantiate-templates` flag is optional, while the other two are necessary (at least the first one), and are usually used together. # PCH_CODEGEN_FLAGS := -fpch-codegen -fpch-debuginfo -fpch-instantiate-templates + +# Guess the amount of RAM we have (in gigabytes), to select an appropriate build profile. +ifneq ($(IS_MACOS),) +ASSUME_RAM := $(shell bash -c 'echo $$(($(shell sysctl -n hw.memsize) / 1000000000))') +else +# `--giga` uses 10^3 instead of 2^10, which is actually good for us, since it overreports a bit, which counters computers typically having slightly less RAM than 2^N gigs. +ASSUME_RAM := $(shell LANG= free --giga 2>/dev/null | gawk 'NR==2{print $$2}') +endif + +ifneq ($(ASSUME_RAM),) +ifeq ($(call safe_shell,echo $$(($(ASSUME_RAM) >= 64))),1) +override ram_string := >= 64G # How many translation units to use for the bindings. Bigger value = less RAM usage, but usually slower build speed. # When changing this, update the default value for `-j` above. +NUM_FRAGMENTS := 8 +# The default number of jobs. Override with `-jN` or `JOBS=N`, both work fine. +JOBS := 8 +else ifeq ($(call safe_shell,echo $$(($(ASSUME_RAM) >= 32))),1) +override ram_string := ~32G +NUM_FRAGMENTS := 16 +JOBS := 8 +else ifeq ($(call safe_shell,echo $$(($(ASSUME_RAM) >= 16))),1) +override ram_string := ~16G +NUM_FRAGMENTS := 64 +JOBS := 8 +else +override ram_string := ~8G (oof) NUM_FRAGMENTS := 64 +JOBS := 4 +endif +else +override ram_string := unknown, assuming ~16G +NUM_FRAGMENTS := 64 +JOBS := 8 +endif +MAKEFLAGS += -j$(JOBS) +ifeq ($(filter-out file,$(origin NUM_FRAGMENTS) $(origin JOBS)),) +$(info RAM $(ram_string), defaulting to NUM_FRAGMENTS=$(NUM_FRAGMENTS) -j$(JOBS)) +else +$(info RAM $(ram_string), NUM_FRAGMENTS=$(NUM_FRAGMENTS) -j$(JOBS)) +endif -# The default number of jobs. Override with `-jN`. -MAKEFLAGS += -j8 # --- End of configuration variables. diff --git a/scripts/mrbind/install_deps_ubuntu.sh b/scripts/mrbind/install_deps_ubuntu.sh index 50f9c2a5228c..b9965608d34d 100755 --- a/scripts/mrbind/install_deps_ubuntu.sh +++ b/scripts/mrbind/install_deps_ubuntu.sh @@ -33,4 +33,5 @@ fi # Install the packages. # Could also add `sudo` here for `install_mrbind_ubuntu.sh`, but I think the user can do that themselves. -apt install -y make cmake ninja-build gawk clang-$CLANG_VER lld-$CLANG_VER clang-tools-$CLANG_VER libclang-$CLANG_VER-dev llvm-$CLANG_VER-dev +# `procps` is for the `free` utility, to measure how much RAM we have. +apt install -y make cmake ninja-build gawk procps clang-$CLANG_VER lld-$CLANG_VER clang-tools-$CLANG_VER libclang-$CLANG_VER-dev llvm-$CLANG_VER-dev diff --git a/scripts/mrbind/install_deps_windows_msys2.bat b/scripts/mrbind/install_deps_windows_msys2.bat index 6ff7ad819ac1..5df6ffe90ed6 100644 --- a/scripts/mrbind/install_deps_windows_msys2.bat +++ b/scripts/mrbind/install_deps_windows_msys2.bat @@ -53,6 +53,7 @@ rem ------ Install a specific version of Clang call %MSYS2_DIR%\msys2_shell.cmd -no-start -defterm -clang64 -c "'%~dp0'/msys2_install_clang_ver.sh %CLANG_VER%" rem ------ Install needed packages -call %MSYS2_DIR%\msys2_shell.cmd -no-start -defterm -clang64 -c "pacman -S --noconfirm --needed make $MINGW_PACKAGE_PREFIX-cmake" +rem `procps-ns` is for the `free` command to measure RAM. +call %MSYS2_DIR%\msys2_shell.cmd -no-start -defterm -clang64 -c "pacman -S --noconfirm --needed make procps-ng $MINGW_PACKAGE_PREFIX-cmake" -endlocal \ No newline at end of file +endlocal diff --git a/scripts/mrbind/mrbind_commit.txt b/scripts/mrbind/mrbind_commit.txt index 7fd7201c907a..1bc56a020d00 100644 --- a/scripts/mrbind/mrbind_commit.txt +++ b/scripts/mrbind/mrbind_commit.txt @@ -1 +1 @@ -dd8cd39c949891539b9c3d830917719ce12798ff +ec31a36432faa8ac78719c0c3023ec24cf5c18ac diff --git a/scripts/mrbind/mrbind_flags.txt b/scripts/mrbind/mrbind_flags.txt index d857331469ed..e02815e6e005 100644 --- a/scripts/mrbind/mrbind_flags.txt +++ b/scripts/mrbind/mrbind_flags.txt @@ -1,5 +1,5 @@ --format=macros ---ignore-pch-flags +--combine-types=cv,ref,ptr,smart_ptr --ignore :: --allow MR --ignore MR::detail diff --git a/scripts/mrbind/mrbind_flags_for_helpers.txt b/scripts/mrbind/mrbind_flags_for_helpers.txt index 27e7ca209900..c00bf094fe64 100644 --- a/scripts/mrbind/mrbind_flags_for_helpers.txt +++ b/scripts/mrbind/mrbind_flags_for_helpers.txt @@ -1,4 +1,4 @@ --format=macros ---ignore-pch-flags +--combine-types=cv,ref,ptr,smart_ptr --ignore :: --allow MR::Extra diff --git a/source/MRMesh/MRObject.h b/source/MRMesh/MRObject.h index ae1396078364..824b0d29f7c0 100644 --- a/source/MRMesh/MRObject.h +++ b/source/MRMesh/MRObject.h @@ -209,7 +209,9 @@ class MRMESH_CLASS Object : public ObjectChildrenHolder /// models in the folder by given path and /// fields in given JSON /// \param childId is its ordinal number within the parent - MRMESH_API Expected>>> serializeRecursive( const std::filesystem::path& path, Json::Value& root, int childId ) const; + // This would be automatically skipped in the bindings anyway because of the `Json::Value` parameter. + // But skipping it here prevents the vector-of-futures type from being registered, which is helpful. + MRMESH_API MR_BIND_IGNORE Expected>>> serializeRecursive( const std::filesystem::path& path, Json::Value& root, int childId ) const; /// loads subtree into this Object /// models from the folder by given path and From a6afbcf873fdd3d0bac325938473907b239094f7 Mon Sep 17 00:00:00 2001 From: Egor Mikhaylov Date: Mon, 25 Nov 2024 11:48:25 +0300 Subject: [PATCH 2/6] Update mrbind. --- scripts/mrbind/mrbind_commit.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/mrbind/mrbind_commit.txt b/scripts/mrbind/mrbind_commit.txt index 1bc56a020d00..762433887ca6 100644 --- a/scripts/mrbind/mrbind_commit.txt +++ b/scripts/mrbind/mrbind_commit.txt @@ -1 +1 @@ -ec31a36432faa8ac78719c0c3023ec24cf5c18ac +ed4cbedf1671851a3782f1a2e19d0a91b72dea2c From 2cff852c3feb40b24bb480770ca80f84a53b44f2 Mon Sep 17 00:00:00 2001 From: Egor Mikhaylov Date: Mon, 25 Nov 2024 13:01:40 +0300 Subject: [PATCH 3/6] Update mrbind again. --- scripts/mrbind/mrbind_commit.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/mrbind/mrbind_commit.txt b/scripts/mrbind/mrbind_commit.txt index 762433887ca6..f431b4d5ebb8 100644 --- a/scripts/mrbind/mrbind_commit.txt +++ b/scripts/mrbind/mrbind_commit.txt @@ -1 +1 @@ -ed4cbedf1671851a3782f1a2e19d0a91b72dea2c +7fb8573234105f7bdd16d5d1a8f5a2d9f666fb79 From 42fd9be8c7fe4c56a8c44a2eb8f822bdc84259f8 Mon Sep 17 00:00:00 2001 From: Egor Mikhaylov Date: Mon, 25 Nov 2024 14:47:01 +0300 Subject: [PATCH 4/6] Add comment. --- source/MRMesh/MRObject.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/MRMesh/MRObject.h b/source/MRMesh/MRObject.h index 824b0d29f7c0..74005fba632b 100644 --- a/source/MRMesh/MRObject.h +++ b/source/MRMesh/MRObject.h @@ -211,6 +211,7 @@ class MRMESH_CLASS Object : public ObjectChildrenHolder /// \param childId is its ordinal number within the parent // This would be automatically skipped in the bindings anyway because of the `Json::Value` parameter. // But skipping it here prevents the vector-of-futures type from being registered, which is helpful. + // TODO: figure out how to automate this (add a flag to the parser to outright reject functions based on their parameter and return types). MRMESH_API MR_BIND_IGNORE Expected>>> serializeRecursive( const std::filesystem::path& path, Json::Value& root, int childId ) const; /// loads subtree into this Object From 3f91b9696434d6e5b62666da56a46bb114ce0e84 Mon Sep 17 00:00:00 2001 From: Egor Mikhaylov Date: Mon, 25 Nov 2024 14:47:27 +0300 Subject: [PATCH 5/6] A clever logic for calculating the build settings. --- scripts/mrbind/generate.mk | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/scripts/mrbind/generate.mk b/scripts/mrbind/generate.mk index d930749cac4a..cf398d11b80c 100644 --- a/scripts/mrbind/generate.mk +++ b/scripts/mrbind/generate.mk @@ -251,32 +251,49 @@ PCH_CODEGEN_FLAGS := # PCH_CODEGEN_FLAGS := -fpch-codegen -fpch-debuginfo -fpch-instantiate-templates + +# --- Guess the build settings for the optimal speed: + # Guess the amount of RAM we have (in gigabytes), to select an appropriate build profile. +# Also guess the number of CPU cores. ifneq ($(IS_MACOS),) ASSUME_RAM := $(shell bash -c 'echo $$(($(shell sysctl -n hw.memsize) / 1000000000))') +ASSUME_NPROC := $(call safe_shell,sysctl -n hw.ncpu) else # `--giga` uses 10^3 instead of 2^10, which is actually good for us, since it overreports a bit, which counters computers typically having slightly less RAM than 2^N gigs. ASSUME_RAM := $(shell LANG= free --giga 2>/dev/null | gawk 'NR==2{print $$2}') +ASSUME_NPROC := $(call safe_shell,nproc) +endif + +# We clamp the nproc to this value, because when you have more cores, our heuristics fall apart (and you might run out of ram). +# The heuristics are not necessarily bad though, it's possible that less cores than jobs can be better in some cases? +MAX_NPROC := 16 +CAPPED_NPROC := $(ASSUME_NPROC) +override nproc_string := $(ASSUME_NPROC) cores +ifeq ($(call safe_shell,echo $$(($(ASSUME_NPROC) >= $(MAX_NPROC)))),1) +CAPPED_NPROC := $(MAX_NPROC) +override nproc_string := >=$(MAX_NPROC) cores endif ifneq ($(ASSUME_RAM),) ifeq ($(call safe_shell,echo $$(($(ASSUME_RAM) >= 64))),1) -override ram_string := >= 64G +override ram_string := >=64G RAM +# The default number of jobs. Override with `-jN` or `JOBS=N`, both work fine. +JOBS := $(CAPPED_NPROC) # How many translation units to use for the bindings. Bigger value = less RAM usage, but usually slower build speed. # When changing this, update the default value for `-j` above. -NUM_FRAGMENTS := 8 -# The default number of jobs. Override with `-jN` or `JOBS=N`, both work fine. -JOBS := 8 +NUM_FRAGMENTS := $(CAPPED_NPROC) else ifeq ($(call safe_shell,echo $$(($(ASSUME_RAM) >= 32))),1) -override ram_string := ~32G -NUM_FRAGMENTS := 16 -JOBS := 8 +override ram_string := ~32G RAM +NUM_FRAGMENTS := $(call safe_shell,echo $$(($(CAPPED_NPROC) * 2)))# = CAPPED_NPROC * 2 +JOBS := $(CAPPED_NPROC) else ifeq ($(call safe_shell,echo $$(($(ASSUME_RAM) >= 16))),1) -override ram_string := ~16G +# At this point we have so little RAM that we ignore nproc completely (or would need to clamp it to something like ~8, but who even has less cores than that?). +override ram_string := ~16G RAM NUM_FRAGMENTS := 64 JOBS := 8 else -override ram_string := ~8G (oof) +override ram_string := ~8G RAM (oof) NUM_FRAGMENTS := 64 JOBS := 4 endif @@ -287,12 +304,14 @@ JOBS := 8 endif MAKEFLAGS += -j$(JOBS) ifeq ($(filter-out file,$(origin NUM_FRAGMENTS) $(origin JOBS)),) -$(info RAM $(ram_string), defaulting to NUM_FRAGMENTS=$(NUM_FRAGMENTS) -j$(JOBS)) +$(info Build machine: $(nproc_string), $(ram_string); defaulting to NUM_FRAGMENTS=$(NUM_FRAGMENTS) -j$(JOBS)) else -$(info RAM $(ram_string), NUM_FRAGMENTS=$(NUM_FRAGMENTS) -j$(JOBS)) +$(info Build machine: $(nproc_string), $(ram_string); NUM_FRAGMENTS=$(NUM_FRAGMENTS) -j$(JOBS))# This can print the wrong `-j` if you override it using `-j` instead of `JOBS=N`. endif + + # --- End of configuration variables. From 53cf9c28b884951c41afb8b21946a91f40958b02 Mon Sep 17 00:00:00 2001 From: Egor Mikhaylov Date: Mon, 25 Nov 2024 15:49:04 +0300 Subject: [PATCH 6/6] Remove parsing tests before the build. --- .github/workflows/build-test-ubuntu-arm64.yml | 7 ------- .github/workflows/build-test-ubuntu-x64.yml | 7 ------- 2 files changed, 14 deletions(-) diff --git a/.github/workflows/build-test-ubuntu-arm64.yml b/.github/workflows/build-test-ubuntu-arm64.yml index 2eff2dcdb0bc..ee5c562159f2 100644 --- a/.github/workflows/build-test-ubuntu-arm64.yml +++ b/.github/workflows/build-test-ubuntu-arm64.yml @@ -104,13 +104,6 @@ jobs: . .venv/bin/activate python3 -m pip install -r ./requirements/python.txt - - name: Check that MRBind bindings parse successfully - if: ${{inputs.mrbind}} - env: - CXX: ${{matrix.cxx-compiler}} - run: | - make -f scripts/mrbind/generate.mk only-generate - - name: Build run: ./scripts/build_source.sh env: diff --git a/.github/workflows/build-test-ubuntu-x64.yml b/.github/workflows/build-test-ubuntu-x64.yml index 70d253b55926..7c42076ce6a3 100644 --- a/.github/workflows/build-test-ubuntu-x64.yml +++ b/.github/workflows/build-test-ubuntu-x64.yml @@ -128,13 +128,6 @@ jobs: - name: Setup python requirements run: python3 -m pip install -r ./requirements/python.txt - - name: Check that MRBind bindings parse successfully - if: ${{inputs.mrbind}} - env: - CXX: ${{matrix.cxx-compiler}} - run: | - make -f scripts/mrbind/generate.mk only-generate - - name: Build run: ./scripts/build_source.sh env: