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

Change mobile binary size comparison workflow to use release build #25189

Merged
merged 2 commits into from
Jan 30, 2023

Conversation

caschoener
Copy link
Contributor

Was debugging a binary size increase in https://github.com/envoyproxy/envoy/actions/runs/4008291380/workflow and noticed that the comparison is based on --config=remote-ci-linux-clang. Talked with JP and it seems more appropriate to compare a release build instead.

Test plan: Look at CI

1
Signed-off-by: caschoener <schoener@google.com>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #25189 was opened by caschoener.

see: more, trace.

@phlax phlax changed the title Change binary size comparison workflow to use release build Change mobile binary size comparison workflow to use release build Jan 26, 2023
@caschoener
Copy link
Contributor Author

image

The number changed from 858 MB to 28. This seems like a more accurate representation of our binary size though. What do you think JP?

@caschoener caschoener marked this pull request as ready for review January 27, 2023 17:03
Signed-off-by: caschoener <schoener@google.com>
Copy link
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

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

I think this is appropriate. Thanks for the fixes!

@jpsim
Copy link
Contributor

jpsim commented Jan 27, 2023

The number changed from 858 MB to 28.

This isn't terribly surprising. On iOS, where we statically link and deadstrip some code at link time, we see a ~5MB size increase from using Envoy Mobile to the compressed single-architecture iOS app.

@caschoener
Copy link
Contributor Author

image

Sweet

@caschoener
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #25189 (comment) was created by @caschoener.

see: more, trace.

@caschoener
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #25189 (comment) was created by @caschoener.

see: more, trace.

@jpsim jpsim merged commit 0b89c59 into envoyproxy:main Jan 30, 2023
VishalDamgude pushed a commit to freshworks/envoy that referenced this pull request Feb 2, 2023
jpsim added a commit that referenced this pull request Feb 7, 2023
This was accidentally disabled in
#25189.

Signed-off-by: JP Simard <jp@jpsim.com>
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.

2 participants