support persistent worker for aar extractors

[SupSaiYaJin]

This PR supports persistent worker for Android aar extractors, it'll significantly improve the aar extractors performance especially in the first time compilation without cache.
It's not enable by default, can use flag build --experimental_persistent_aar_extractor=true to enable it.

using worker:

not using worker:

[ted-xie]

A few general comments:

1. If we're adding a new execution mode for aar extraction, we need to add a test for it. Please see how other tools (i.e. busybox, desguar, dex) test their worker/multiplex worker functionality. I think a lot of those tests are in src/test/bazel/android/android_integration_test.sh.
2. Please add more comments describing what your changes are doing, since behavior around execution strategy is critical functionality, and we want the logic to be as clear as possible.
3. In general the json_worker_wrapper.py file seems quite verbose and low-level for a python program. I would imagine that a lot of this functionality can be refactored to be more pythonic.

[ted-xie]

I'm also curious about what the cumulative time savings of all aar extraction actions is for your change. Your screenshot shows a 758 -> 15 ms wall time reduction, which is indeed impressive, but I think that's just for a single action.

[SupSaiYaJin]

I'm also curious about what the cumulative time savings of all aar extraction actions is for your change. Your screenshot shows a 758 -> 15 ms wall time reduction, which is indeed impressive, but I think that's just for a single action.

It saves nearly 100 seconds for the first-time compilation without cache. It had nearly 2000 extractor actions executed for my project, each of which saved nearly 500ms. I run it on a 10-core CPU, 500ms * 2000 / 10 ≈ 100s

[ted-xie]

Some minor changes requested for this iteration. Looking much better.

[SupSaiYaJin]

When will this PR be merged? Do I need to do anything else? :) @ted-xie

[sgowroji]

Hi @SupSaiYaJin, I will update you once it's merged. Thanks!

[ted-xie]

Hi @SupSaiYaJin, we ran into a couple linting issues while importing these. Could you take a look at the comments and adress them? Thank you!

[ted-xie]

@SupSaiYaJin Stepping back a little, I'm curious what are your thoughts on using a faster unzip implementation, versus implementing worker support for AAR extraction. We could use this as an opportunity to refactor AAR extraction in general to use a different zip inflate implementation, versus adding significant complexity (new flag, new execution strategy) to AAR extraction which in general is not supposed to be the bottleneck on build workflows.

[SupSaiYaJin]

@ted-xie What would this implementation look like? If it also needs to start a new process to run, I think it still needs to run in worker mode.

[ted-xie]

Sorry, I missed the notification for this PR.

What would this implementation look like?

Jar/aar extraction is basically just unzip; if you choose a faster unzip executable, the combined process call + execution time may be less than the performance gain from worker mode.

If it also needs to start a new process to run, I think it still needs to run in worker mode.

Much of the current performance penalty for this action comes from having to spin up a java process, i.e. the entire JVM needs to come alive. The same limitation does not exist for unzip tools written in a non-runtime language.

[SupSaiYaJin]

@ted-xie We can use worker for now and do it in the future, I think it's a good idea :).
Anything else I should do with this pr?

[sgowroji]

Hi @SupSaiYaJin , Could you please update the latest changes from master into the branch as it is awaiting to merge. Thanks!

[SupSaiYaJin]

@sgowroji done

[SupSaiYaJin]

@ted-xie done

[SupSaiYaJin]

@ted-xie Do you use any instant messaging apps? I would like to have a more in-depth conversation with you. Could we add each other as friends?