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

Attempt to fix JVM high memory usage in CI #1331

Merged
merged 1 commit into from
Feb 7, 2025
Merged

Conversation

PIG208
Copy link
Member

@PIG208 PIG208 commented Feb 5, 2025

Will trigger the CI with this.

@PIG208 PIG208 marked this pull request as ready for review February 5, 2025 22:51
@PIG208 PIG208 force-pushed the pr-build branch 2 times, most recently from 91fe630 to cd2b5d5 Compare February 7, 2025 02:18
@PIG208 PIG208 requested a review from gnprice February 7, 2025 02:19
@PIG208 PIG208 added the integration review Added by maintainers when PR may be ready for integration label Feb 7, 2025
We are pretty certain that the upstream commit
  flutter/flutter@df114fbe9
was the exact one that caused the increased RAM usage for the Android
build.

The upstream change causes the engine artifacts to contain debug
symbols, which is intentional (as it enables putting debug symbols in
the app's bundle artifact) but makes the artifacts about 8x larger.
That causes known regressions in CPU and memory use at build time:
  flutter/flutter#162675

Since the regression was expected, there is no action to be taken
upstream.

However, we haven't found an effective way to rewrite the build script in
a way that this is mitigated without needing to raise the limit. For the
investigation details, see CZO discussion:
  https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/Gradle.20out.20of.20memory

Since a previous bump to 3 GiB, the issue has been mitigated, but it
still happens some of the time: at least twice in the past day or so.
Add another 1 GiB to see if that addresses the flakes.
@gnprice
Copy link
Member

gnprice commented Feb 7, 2025

Thanks for all the investigation!

Pushed a revision that adjusts the commit message in a few places, and that also makes the diff do the same number the commit message says 🙂 (it was actually reverting the previous bump).

I'll let CI run on that to be sure I haven't missed something, and then merge.

@gnprice gnprice merged commit c66a905 into zulip:main Feb 7, 2025
1 check passed
@PIG208 PIG208 deleted the pr-build branch February 7, 2025 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants