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.

Sign up for GitHub

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

Jump to bottom

chore(dev, releasing): Have Cargo.toml release profile match what releases are published with #20034

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

chore(dev, releasing): Have Cargo.toml release profile match what releases are published with #20034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
03456ab
chore(dev, releasing): Have Cargo.toml `release` profile match what r…
jszwedko Mar 7, 2024
7d2cbbd
Fix CI errors
jszwedko Mar 11, 2024
ec1efd8
Enable building dev packages
jszwedko Mar 11, 2024
6f7673d
build dev cross profile
jszwedko Mar 11, 2024
15269c9
debug isn't a real profile
jszwedko Mar 12, 2024
8270333
Merge remote-tracking branch 'origin/master' into jszwedko/release-pr…
jszwedko Mar 12, 2024
5720ff3
default env vars
jszwedko Mar 12, 2024
cf51de7
Try to correct target
jszwedko Mar 12, 2024
4f73c5d
Set target subdir
jszwedko Mar 12, 2024
bbac36a
fix profiles
jszwedko Mar 12, 2024
602e6e9
Fix cross profile
jszwedko Mar 22, 2024
ddf822c
debugging
jszwedko Mar 22, 2024
d150b1f
Fix targets
jszwedko Mar 22, 2024
e40f048
Merge remote-tracking branch 'origin/master' into jszwedko/release-pr…
jszwedko Mar 22, 2024
ec661bb
Fix targets
jszwedko Mar 22, 2024
63cc679
Merge remote-tracking branch 'origin/master' into jszwedko/release-pr…
jszwedko Oct 31, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/ci-integration-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev

jobs:
prep-pr:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-review-trigger.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev
# observing issues fetching boringssl via HTTPS in the OSX build, seeing if this helps
# can be removed when we switch back to the upstream openssl-sys crate
CARGO_NET_GIT_FETCH_WITH_CLI: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/compilation-timings.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
# We're not actually doing a debug build, we're just turning off the logic
# in release-flags.sh so that we don't override the Cargo "release" profile
# with full LTO / single codegen unit.
PROFILE: debug
PROFILE: dev
steps:
- uses: colpal/actions-clean@v1
- uses: actions/checkout@v4
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/cross.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ jobs:
timeout-minutes: 45
env:
CARGO_INCREMENTAL: 0
PROFILE: dev
strategy:
matrix:
target:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev
# observing issues fetching boringssl via HTTPS in the OSX build, seeing if this helps
# can be removed when we switch back to the upstream openssl-sys crate
CARGO_NET_GIT_FETCH_WITH_CLI: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev

jobs:
test-integration:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev
# observing issues fetching boringssl via HTTPS in the OSX build, seeing if this helps
# can be removed when we switch back to the upstream openssl-sys crate
CARGO_NET_GIT_FETCH_WITH_CLI: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/k8s_e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ env:
VERBOSE: true
DISABLE_MOLD: true
CI: true
PROFILE: debug
PROFILE: dev

jobs:
changes:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/master_merge_queue.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev
# observing issues fetching boringssl via HTTPS in the OSX build, seeing if this helps
# can be removed when we switch back to the upstream openssl-sys crate
CARGO_NET_GIT_FETCH_WITH_CLI: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/msrv.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
env:
RUST_BACKTRACE: full
CI: true
PROFILE: debug
PROFILE: dev
# observing issues fetching boringssl via HTTPS in the OSX build, seeing if this helps
# can be removed when we switch back to the upstream openssl-sys crate
CARGO_NET_GIT_FETCH_WITH_CLI: true
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/regression.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ jobs:
build-baseline:
name: Build baseline Vector container
runs-on: ubuntu-20.04-4core
timeout-minutes: 30
timeout-minutes: 60
needs:
- should-run-gate
- resolve-inputs
Expand Down Expand Up @@ -224,7 +224,7 @@ jobs:
build-comparison:
name: Build comparison Vector container
runs-on: ubuntu-20.04-4core
timeout-minutes: 30
timeout-minutes: 60
needs:
- should-run-gate
- resolve-inputs
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ env:
TEST_LOG: vector=debug
VERBOSE: true
CI: true
PROFILE: debug
PROFILE: dev
# observing issues fetching boringssl via HTTPS in the OSX build, seeing if this helps
# can be removed when we switch back to the upstream openssl-sys crate
CARGO_NET_GIT_FETCH_WITH_CLI: true
Expand Down
14 changes: 10 additions & 4 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,17 @@ path = "tests/integration/lib.rs"
name = "e2e"
path = "tests/e2e/mod.rs"

# CI-based builds use full release optimization. See scripts/environment/release-flags.sh.
# This results in roughly a 5% reduction in performance when compiling locally vs when
# compiled via the CI pipeline.
[profile.release]
debug = false # Do not include debug symbols in the executable.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the default so I dropped it.

lto = true
Copy link

@polarathene polarathene Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is "fat" worthwhile vs "thin"? Technically there's a few variants to choose.

  • false (default) will opt-out of LTO when codegen-units=1 or opt-level = 0, otherwise it does crate local thin LTO (slightly different than "thin").
  • "thin" => Thin LTO
  • "fat" / true => Full LTO (much slower, barely any notable gain)
  • "off" => Disable LTO

In my testing with build size and time, I didn't see much value from building Vector with "fat" / true. It was notably slower than false / "thin" due to single thread vs multi-thread CPU usage at link time.

From what I've read of those that profile between thin and fat/full LTO there's barely a statistical difference in improvement to warrant the excess build time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good thought. Once this is merged, we can benchmark against thin and see what the difference looks like for fat vs. thin. I believe we saw a 5% difference in throughput for fat vs false.

Copy link

@polarathene polarathene Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I found this blogpost that explains the difference for lto = false (thin local LTO) vs lto ="thin", where false does thin LTO over each crates codegen units.

I believe we saw a 5% difference in throughput for fat vs false.

That's interesting, would be good to know how that compes to fat vs thin 👍

Was the false comparison done with codegen-units = 1? Because as I mentioned earlier, that would opt-out of LTO and be equivalent to lto = "false".

References:

https://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html

ThinLTO already performs well compared to LTO, in many cases matching the performance improvement.
In a few cases ThinLTO even outperforms full LTO, most likely because the higher scalability of ThinLTO allows using a more aggressive backend optimization pipeline (similar to that of a non-LTO build).

https://convolv.es/guides/lto/

our recurring dataset shows a 0.23% run-time delta between LLVM’s basic and parallel modes
LLVM’s implementation of parallel LTO achieves nearly all of the run-time performance improvement as seen with basic LTO:

  • basic LTO reached a 2.86% improvement over non-LTO
  • while parallel LTO achieved a 2.63% improvement over non-LTO.

Not sure how much it differs between GCC vs Clang as the linker driver. I think with Clang you need to have a compatible version to the Rust toolchain used (at least if doing cross-language LTO) which may not work as well with the CI image builds using cross docker images (Ubuntu)? 🤷‍♂️ (this comment seems to claim a 20% runtime perf improvement by changing linker from GCC to Clang)

codegen-units = 1

# This profile is useful for local performance testing. It results in roughly a 5% reduction in
# performance compared with the full release profile but compiles much more quickly.
[profile.dev-perf]
inherits = "release"
lto = false
codegen-units = 16
bruceg marked this conversation as resolved.
Show resolved Hide resolved
debug = true

[profile.bench]
debug = true
Expand Down
30 changes: 22 additions & 8 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ help:
@printf -- "\n"
@awk 'BEGIN {FS = ":.*##"; printf "Usage: make ${FORMATTING_BEGIN_BLUE}<target>${FORMATTING_END}\n"} /^[a-zA-Z0-9_-]+:.*?##/ { printf " ${FORMATTING_BEGIN_BLUE}%-46s${FORMATTING_END} %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)

define target_to_profile
$(if $(filter-out debug,$(1)),$(1),dev)
endef

define profile_to_target
$(if $(filter-out dev,$(1)),$(1),debug)
endef

##@ Environment

# These are some predefined macros, please use them!
Expand Down Expand Up @@ -272,31 +280,36 @@ cross-%: export CFLAGS += -g0 -O3
cross-%: cargo-install-cross
$(MAKE) -k cross-image-${TRIPLE}
cross ${COMMAND} \
$(if $(findstring release,$(PROFILE)),--release,) \
--profile $(PROFILE) \
--target ${TRIPLE} \
--no-default-features \
--features target-${TRIPLE}

target/%/vector: export PAIR =$(subst /, ,$(@:target/%/vector=%))
target/%/vector: export TRIPLE ?=$(word 1,${PAIR})
target/%/vector: export PROFILE ?=$(word 2,${PAIR})
target/%/vector: export TARGET_SUBDIR ?=$(word 2,${PAIR})
target/%/vector: export PROFILE ?=$(if $(filter-out debug,$(TARGET_SUBDIR)),$(TARGET_SUBDIR),dev)
target/%/vector: export CFLAGS += -g0 -O3
target/%/vector: cargo-install-cross CARGO_HANDLES_FRESHNESS
echo $(PAIR)
echo $(TARGET_SUBDIR)
echo $(PROFILE)
$(MAKE) -k cross-image-${TRIPLE}
cross build \
$(if $(findstring release,$(PROFILE)),--release,) \
--profile $(PROFILE) \
--target ${TRIPLE} \
--no-default-features \
--features target-${TRIPLE}

target/%/vector.tar.gz: export PAIR =$(subst /, ,$(@:target/%/vector.tar.gz=%))
target/%/vector.tar.gz: export TRIPLE ?=$(word 1,${PAIR})
target/%/vector.tar.gz: export PROFILE ?=$(word 2,${PAIR})
target/%/vector.tar.gz: export TARGET_SUBDIR ?=$(word 2,${PAIR})
target/%/vector.tar.gz: export PROFILE ?=$(if $(filter-out debug,$(TARGET_SUBDIR)),$(TARGET_SUBDIR),dev)
target/%/vector.tar.gz: target/%/vector CARGO_HANDLES_FRESHNESS
rm -rf target/scratch/vector-${TRIPLE} || true
mkdir -p target/scratch/vector-${TRIPLE}/bin target/scratch/vector-${TRIPLE}/etc
cp -R -f -v \
target/${TRIPLE}/${PROFILE}/vector \
target/${TRIPLE}/${TARGET_SUBDIR}/vector \
target/scratch/vector-${TRIPLE}/bin/vector
cp -R -f -v \
README.md \
Expand All @@ -312,7 +325,7 @@ target/%/vector.tar.gz: target/%/vector CARGO_HANDLES_FRESHNESS
tar --create \
--gzip \
--verbose \
--file target/${TRIPLE}/${PROFILE}/vector.tar.gz \
--file target/${TRIPLE}/${TARGET_SUBDIR}/vector.tar.gz \
--directory target/scratch/ \
./vector-${TRIPLE}
rm -rf target/scratch/
Expand Down Expand Up @@ -510,8 +523,9 @@ build-rustdoc: ## Build Vector's Rustdocs

# archives
target/artifacts/vector-${VERSION}-%.tar.gz: export TRIPLE :=$(@:target/artifacts/vector-${VERSION}-%.tar.gz=%)
target/artifacts/vector-${VERSION}-%.tar.gz: override PROFILE =release
target/artifacts/vector-${VERSION}-%.tar.gz: target/%/release/vector.tar.gz
target/artifacts/vector-${VERSION}-%.tar.gz: export PROFILE ?= release
target/artifacts/vector-${VERSION}-%.tar.gz: export TARGET_SUBDIR ?=$(if $(filter-out dev,$(PROFILE)),$(PROFILE),debug)
target/artifacts/vector-${VERSION}-%.tar.gz: target/%/$(TARGET_SUBDIR)/vector.tar.gz
@echo "Built to ${<}, relocating to ${@}"
@mkdir -p target/artifacts/
@cp -v \
Expand Down
3 changes: 0 additions & 3 deletions scripts/environment/prepare.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,3 @@ fi
# ci image to install a newer version of node.
sudo npm -g install [email protected]
sudo npm -g install @datadog/datadog-ci

# Make sure our release build settings are present.
. scripts/environment/release-flags.sh
14 changes: 0 additions & 14 deletions scripts/environment/release-flags.sh
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion scripts/test-e2e-kubernetes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ if [[ -z "${CONTAINER_IMAGE:-}" ]]; then
else
# Package a .deb file to build a docker container, unless skipped.
if [[ -z "${SKIP_PACKAGE_DEB:-}" ]]; then
make package-deb-x86_64-unknown-linux-gnu
make package-deb-x86_64-unknown-linux-gnu PROFILE=dev
fi

# Prepare test image parameters.
Expand Down
Loading