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

build: shrink size of Rust artifacts #2696

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

morrisonlevi
Copy link
Collaborator

@morrisonlevi morrisonlevi commented Jun 7, 2024

PROF-9904

Description

This is an attempt to shrink artifacts, but unlike #2601, this does not do panic=abort. For panic=abort to be really effective, you also need to rebuild std from source. That means rebuilding images and also installing Rust from rustup (because the packages we are using do not include std sources). We had issues with CI last time we tried it. We played whack-a-mole for a while, but since the issues just kept going and we wanted to release 1.0.0beta1, we decide to revert back.

But this PR without panic=abort will still shrink the size a fair bit, so it's worth splitting out. I wouldn't go so far as saying that it will fix the size issue reported in PR #2599, but it helps.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@morrisonlevi morrisonlevi requested a review from a team as a code owner June 7, 2024 02:01
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.32%. Comparing base (e636250) to head (76dd2bf).
Report is 2 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2696      +/-   ##
============================================
+ Coverage     77.77%   79.32%   +1.55%     
  Complexity     2223     2223              
============================================
  Files           226      200      -26     
  Lines         26386    22369    -4017     
  Branches        988        0     -988     
============================================
- Hits          20522    17745    -2777     
+ Misses         5338     4624     -714     
+ Partials        526        0     -526     
Flag Coverage Δ
appsec-extension ?
tracer-extension 78.62% <ø> (ø)
tracer-php 80.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 26 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e636250...76dd2bf. Read the comment docs.

This does not do panic=abort because to be effective, you also need
to rebuild std, and _that_ means rebuilding images and also installing
from rustup (because the packages we are using do not include std
sources).

But this will still shrink the size a fair bit.
@morrisonlevi
Copy link
Collaborator Author

Difference between the commit on master and this branch:

# This branch
-rw-r--r-- 1 levi.morrison staff 425M Jun  6 21:30 dd-library-php-1.0.0+76dd2bf00d17575ba6c41682e29f419dd7795668-aarch64-linux-gnu.tar.gz
-rw-r--r-- 1 levi.morrison staff 428M Jun  6 21:30 dd-library-php-1.0.0+76dd2bf00d17575ba6c41682e29f419dd7795668-x86_64-linux-gnu.tar.gz

# master
-rw-r--r-- 1 levi.morrison staff 496M Jun  6 20:14 dd-library-php-1.0.0+e63625078edc52eb92e4c6104f0daaab3ec8fa63-aarch64-linux-gnu.tar.gz
-rw-r--r-- 1 levi.morrison staff 500M Jun  6 20:11 dd-library-php-1.0.0+e63625078edc52eb92e4c6104f0daaab3ec8fa63-x86_64-linux-gnu.tar.gz

Roughly 15% savings.

@pr-commenter
Copy link

pr-commenter bot commented Jun 7, 2024

Benchmarks

Benchmark execution time: 2024-06-07 03:46:29

Comparing candidate commit 76dd2bf in PR branch levi/shrink-panic-unwind with baseline commit e636250 in branch master.

Found 2 performance improvements and 1 performance regressions! Performance is the same for 26 metrics, 7 unstable metrics.

scenario:php-profiler-timeline-memory-with-profiler-and-timeline

  • 🟩 cpu_user_time [-110.046ms; -61.233ms] or [-5.017%; -2.792%]
  • 🟩 execution_time [-43.920ms; -27.745ms] or [-3.719%; -2.349%]

scenario:walk_stack/1

  • 🟥 wall_time [+509.800ns; +535.407ns] or [+2.161%; +2.269%]

@realFlowControl realFlowControl merged commit 48e857d into master Jun 7, 2024
556 checks passed
@realFlowControl realFlowControl deleted the levi/shrink-panic-unwind branch June 7, 2024 06:41
@github-actions github-actions bot added this to the 1.1.0 milestone Jun 7, 2024
morrisonlevi added a commit that referenced this pull request Jun 10, 2024
This does not do panic=abort because to be effective, you also need
to rebuild std, and _that_ means rebuilding images and also installing
from rustup (because the packages we are using do not include std
sources).

But this will still shrink the size a fair bit.
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.

3 participants