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

Reduce AndroidAssetMerger intermediate outputs #11303

Closed
wants to merge 1 commit into from

Conversation

jongerrish
Copy link
Contributor

They are not required in the final build.

For our builds:-

This results in 503MB of outputs (assets.zip files) - each of these includes the transitive closure of all assets. To put in perspective we have only 2.5MB of assets.

This is very costly for remote builds with caches especially on slower connections

If resource conflict checking is not enabled (it is not for our build) this will also avoid running AndroidAssetMerger all together saving 896 actions ~3mins total CPU time.

bazelbuild/rules_android#10

…t required in the final build. For our builds:-

This results in 503MB of outputs (assets.zip files) - each of these includes the transitive closure of all assets. To put in perspective we have only 2.5MB of assets.

If resource conflict checking is not enabled (it is not for our build) this will also avoid running AndroidAssetMerger all together saving 896 actions ~3mins total CPU time.
@jin
Copy link
Member

jin commented May 7, 2020

cc @timpeut @djwhang, PTAL?

@aiuto aiuto added the team-Android Issues for Android team label May 13, 2020
@aiuto
Copy link
Contributor

aiuto commented May 13, 2020

reviewers: This should be an easy thing to pick off in the issue fixit.

@ulfjack
Copy link
Contributor

ulfjack commented May 20, 2020

@jin, there's been no reply here for ~two weeks. Any way we can move this forward?

@philwo
Copy link
Member

philwo commented Oct 21, 2020

@lberki Friendly ping? 😅

@lberki
Copy link
Contributor

lberki commented Oct 21, 2020

We don't have capacity to review changes for Android rules at the moment.

@keith
Copy link
Member

keith commented Jan 22, 2021

@jin @lberki has anything changed here? We'd love to reduce the amount of data we're having to shuttle here

@lberki
Copy link
Contributor

lberki commented Jan 25, 2021

@ahumesky ?

@nkoroste
Copy link
Contributor

Any updates on this?

@Bencodes
Copy link
Contributor

Bencodes commented Jun 9, 2021

@jin @lberki @ahumesky has anything changed here? Anything we can do to help get this merged?

@ckolli5 ckolli5 added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 26, 2022
@ahumesky ahumesky removed request for jin and lberki July 25, 2022 15:42
@ted-xie
Copy link
Contributor

ted-xie commented Jul 25, 2022

I'm shepherding this. After fixing a few merge conflicts, looks like no red flags in internal tests so far. There are a few nits that I can take of internally on my end.

@ted-xie ted-xie self-assigned this Jul 25, 2022
@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.