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

Improve binding build times. #3738

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/build-test-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-test-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pip-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/prepare-images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions scripts/mrbind/compiler_only_flags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
40 changes: 38 additions & 2 deletions scripts/mrbind/generate.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Comment on lines +267 to +292
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use more than 8 jobs if we can?

endif

# The default number of jobs. Override with `-jN`.
MAKEFLAGS += -j8

# --- End of configuration variables.

Expand Down
3 changes: 2 additions & 1 deletion scripts/mrbind/install_deps_ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions scripts/mrbind/install_deps_windows_msys2.bat
Original file line number Diff line number Diff line change
Expand Up @@ -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
endlocal
2 changes: 1 addition & 1 deletion scripts/mrbind/mrbind_commit.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
dd8cd39c949891539b9c3d830917719ce12798ff
ec31a36432faa8ac78719c0c3023ec24cf5c18ac
2 changes: 1 addition & 1 deletion scripts/mrbind/mrbind_flags.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--format=macros
--ignore-pch-flags
--combine-types=cv,ref,ptr,smart_ptr
--ignore ::
--allow MR
--ignore MR::detail
Expand Down
2 changes: 1 addition & 1 deletion scripts/mrbind/mrbind_flags_for_helpers.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
--format=macros
--ignore-pch-flags
--combine-types=cv,ref,ptr,smart_ptr
--ignore ::
--allow MR::Extra
4 changes: 3 additions & 1 deletion source/MRMesh/MRObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::vector<std::future<Expected<void>>>> 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<std::vector<std::future<Expected<void>>>> serializeRecursive( const std::filesystem::path& path, Json::Value& root, int childId ) const;
Comment on lines +213 to +214
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to think how to prevent this behavior mb add TODO


/// loads subtree into this Object
/// models from the folder by given path and
Expand Down
Loading