Skip to content

Commit

Permalink
Reland PR #1900 "Add platform-specific release configuration"
Browse files Browse the repository at this point in the history
The original PR failed to apply the `release` configuration to
release_unix, leading to a binary size regression, especially on macOS.
This was caused by bazel using fastbuild mode instead of opt mode,
Rust code was not being optimized and on macOS -DDEBUG was defined
through the bazel toolchain defaults.

- Fix a typo and clarify Windows release config comment

Original commit message:
- Match macOS wd_benchmark link options with wd_binary options
- Correct spelling of WIN32_LEAN_AND_MEAN
- /await and /GF are ignored by clang-cl, no need to pass them in the
  Windows build.
  • Loading branch information
fhanau committed Apr 7, 2024
1 parent 789638f commit 187f150
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 20 deletions.
55 changes: 40 additions & 15 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,8 @@ build --@rules_rust//:extra_exec_rustc_flags=-C,panic=abort,-C,debug-assertions=
build:fastdbg --@rules_rust//:extra_rustc_flags=-C,panic=unwind,-C,debug-assertions=y,-C,debuginfo=1,-C,embed-bitcode=n
build:fastdbg --@rules_rust//:extra_exec_rustc_flags=-C,panic=unwind,-C,debug-assertions=y,-C,debuginfo=1,-C,embed-bitcode=n

# TODO(later): -C codegen-units=1 improves code size and quality, should be enabled in a future
# release configuration even if lto is off. Similarly, adding -C lto=thin would improve binary size
# significantly – disable it for now due to compile errors and wrong code generation when bazel and
# rust use different LLVM versions.
# Adding -C lto=thin here would improve binary size significantly – disable it for now due to
# compile errors and wrong code generation when bazel and rust use different LLVM versions.
build:thin-lto --@rules_rust//:extra_rustc_flags=-C,panic=abort,-C,codegen-units=1,-C,embed-bitcode=n

# common sanitizers options
Expand Down Expand Up @@ -178,13 +176,6 @@ build:linux --linkopt="-Wl,--gc-sections" --linkopt="-Wl,-O2"
build:linux --cxxopt="-ffunction-sections" --host_cxxopt="-ffunction-sections"
build:linux --cxxopt="-fdata-sections" --host_cxxopt="-fdata-sections"

# TODO(later): rustc's -C,codegen-units=1 and ld/lld's -Wl,-O2 on Linux/possibly Windows should
# bring an additional binary size improvement. These should only be enabled in opt mode which won't
# be possible in bazelrc without establishing a release configuration, but it can be done using the
# extra_rustc_flags parameter in rust_register_toolchains() and by modifying linkopts in kj_test()
# and wd_cc_binary() similar to how the use_dead_strip flag is used. The binary size improvement is
# modest (~ 300kb), but if we encounter size concerns again this would be a good place to start.

#
# Windows
#
Expand Down Expand Up @@ -215,19 +206,53 @@ build:windows --extra_execution_platforms=//:x64_windows-clang-cl
build:windows_no_dbg -c opt
build:windows_no_dbg --copt='-O0' --host_copt='-O0'
build:windows_no_dbg --copt='/Od' --host_copt='/Od'
build:windows_no_dbg --copt='/INCREMENTAL:NO' --host_copt='/INCREMENTAL:NO'
build:windows_no_dbg --linkopt='/INCREMENTAL:NO' --host_linkopt='/INCREMENTAL:NO'
build:windows_no_dbg --features=-smaller_binary --features=-disable_assertions_feature

# Mitigate the large size impact of Windows debug binaries somewhat by applying string merging and
# linker garbage collection.
build:windows_dbg --config=debug
build:windows_dbg --copt='/Gy' --copt='/Gw' --copt='/GF'
build:windows_dbg --copt='/Gy' --copt='/Gw'
build:windows_dbg --linkopt='/OPT:REF'
build:windows_dbg --linkopt='/OPT:LLDTAILMERGE' --linkopt='/OPT:SAFEICF'

# This hides inline symbols in classes that are marked to be exported, similar to
# -fvisibility-inlines-hidden on Unix systems (https://blog.llvm.org/2018/11/30-faster-windows-builds-with-clang-cl_14.html)
# Currently this only reduces object sizes slightly, larger gains are possible if we compile V8 as
# a shared library.
build:windows --copt='/Zc:dllexportInlines-'

# Release configuration.
build:release -c opt
build:release --@rules_rust//:extra_rustc_flags=-C,panic=abort,-C,codegen-units=1,-C,embed-bitcode=n

# enable -O3 for the release configuration. Based on benchmarking there is little difference in
# performance, but -O3 should generally be expected to be faster for at least parts of the workerd API.
build:release_unix --copt='-O3'
build:release_unix --config=release
build:release_linux --config=release_unix

build:release_macos --config=release_unix
# Disable generating LC_DATA_IN_CODE and LC_FUNCTION_STARTS binary sections, two rarely used types
# of macOS debug info. These sections are largely undocumented, but are used by LLDB to improve
# debugging on binaries that are otherwise stripped. There should be no need to include this
# data in releases.
build:release_macos --linkopt="-Wl,-no_data_in_code_info"
build:release_macos --linkopt="-Wl,-no_function_starts"

build:release_windows --config=release
# Windows uses /O2 as its preferred optimization setting and enabled by bazel in the opt
# configuration, but for clang-cl this is equivalent to only -O2 and a few other things. -O3 is
# not exposed directly in the clang-cl driver, but we can access regular clang
# flags using the /clang prefix anyway. https://clang.llvm.org/docs/UsersManual.html#the-clang-option
build:release_windows --copt="/clang:-O3"
# clang-cl does not enable strict aliasing by default to match MSVC's approach, unlike clang on
# Unix which turns it on for opt builds.
build:release_windows --copt="-fstrict-aliasing"

build:windows --cxxopt='/std:c++20' --host_cxxopt='/std:c++20'
build:windows --cxxopt='/await' --host_cxxopt='/await'
build:windows --copt='/D_CRT_USE_BUILTIN_OFFSETOF' --host_copt='/D_CRT_USE_BUILTIN_OFFSETOF'
build:windows --copt='/DWINDOWS_LEAN_AND_MEAN' --host_copt='/DWINDOWS_LEAN_AND_MEAN'
build:windows --copt='/DWIN32_LEAN_AND_MEAN' --host_copt='/DWIN32_LEAN_AND_MEAN'
# The `/std:c++20` argument is unused during boringssl compilation and we don't
# want a warning when compiling each file.
build:windows --copt='-Wno-unused-command-line-argument' --host_copt='-Wno-unused-command-line-argument'
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,13 @@ jobs:
include:
- os-name: linux
os: ubuntu-20.04
bazel-config: release_linux
- os-name: macOS
os: macos-13
bazel-config: release_macos
- os-name: windows
os: windows-2022
bazel-config: release_windows
runs-on: ${{ matrix.os }}
name: build (${{ matrix.os-name }})
steps:
Expand Down Expand Up @@ -106,7 +109,7 @@ jobs:
fi
- name: Bazel build
run: |
bazelisk build --disk_cache=~/bazel-disk-cache --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev -c opt //src/workerd/server:workerd
bazelisk build --disk_cache=~/bazel-disk-cache --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=${{ matrix.bazel-config }} //src/workerd/server:workerd
- name: Strip debug symbols
if: runner.os != 'Windows'
run: |
Expand Down
7 changes: 6 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ jobs:
# Not enabled because symbolication does not work on workerd macOS builds yet and running
# llvm-symbolizer in the currently broken state causes some tests to time out on the
# runner.
# Use latest available Xcode version – runner still defaults to 14.3.1.
# Use latest available Xcode version – runner still defaults to 15.0.1.
run: |
sudo xcode-select -s "/Applications/Xcode_15.1.app"
# export LLVM_SYMBOLIZER=$(brew --prefix llvm@15)/bin/llvm-symbolizer
Expand All @@ -115,12 +115,17 @@ jobs:
# don't need to run within a debugger, so information needed for symbolication is
# sufficient. The host configuration compiles in opt mode/without debug info by default, so
# there's no need to set host_copt here.
# LLVM produces a bit more debug info on macOS by default to facilitate debugging with
# LLDB. This is not needed for this use case, so disable it using -fno-standalone-debug –
# this is already the default for Linux/Windows.
if: contains(matrix.config.suffix, 'debug') || contains(matrix.config.suffix, 'asan')
shell: bash
run: |
cat <<EOF >> .bazelrc
build:limit-storage --build_tag_filters=-off-by-default,-benchmark
build:limit-storage --copt="-g1"
build:limit-storage --copt="-fno-standalone-debug"
build:limit-storage --@rules_rust//:extra_rustc_flags=-C,panic=unwind,-C,debug-assertions=y,-C,debuginfo=limited,-C,embed-bitcode=n
build:asan --config=limit-storage
build:debug --config=limit-storage
EOF
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.release
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ COPY .bazel-cache /bazel-disk-cache
RUN npm install -g pnpm@latest-7
RUN pnpm install

RUN pnpm exec bazelisk build --disk_cache=/bazel-disk-cache -c opt //src/workerd/server:workerd
RUN pnpm exec bazelisk build --disk_cache=/bazel-disk-cache --config=release_linux //src/workerd/server:workerd

FROM scratch as artifact
COPY --from=builder /workerd/bazel-bin/src/workerd/server/workerd /workerd-linux-arm64
Expand Down
2 changes: 1 addition & 1 deletion build-releases.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ TAG_NAME=$(curl -sL https://api.github.com/repos/cloudflare/workerd/releases/lat
git checkout $TAG_NAME

# Build macOS binary
pnpm exec bazelisk build --disk_cache=./.bazel-cache -c opt //src/workerd/server:workerd
pnpm exec bazelisk build --disk_cache=./.bazel-cache --config=release_macos //src/workerd/server:workerd

cp bazel-bin/src/workerd/server/workerd ./workerd-darwin-arm64

Expand Down
2 changes: 1 addition & 1 deletion build/wd_cc_benchmark.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def wd_cc_benchmark(
"//conditions:default": 1,
}),
linkopts = linkopts + select({
"@//:use_dead_strip": ["-Wl,-dead_strip"],
"@//:use_dead_strip": ["-Wl,-dead_strip", "-Wl,-no_exported_symbols"],
"//conditions:default": [""],
}),
visibility = visibility,
Expand Down

0 comments on commit 187f150

Please sign in to comment.