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

Feature: Parallel Building #3476

Merged
merged 5 commits into from
Dec 26, 2023
Merged

Feature: Parallel Building #3476

merged 5 commits into from
Dec 26, 2023

Conversation

Cregrant
Copy link
Contributor

Here are my performance optimizations that aim to minimize changes to the codebase and do not use any external libraries (although Apache Compress seems like a nice one).

  • There is some duplicated code, such as the ApkBuilder and ApkDecoder new fields, which can be extracted to a "global" context created somewhere in the Main class. However, I do not want to make significant changes for just one pull request.
  • I did not find any changes in the decompiled code, compression type issues, or any other problems.
  • Further performance optimizations can be done by changing the zip compressing library and refactoring the backsmali code (specifically, the synchronized method inside the library).

Best numbers (30mb apk, 3 dex files, 4 CPU cores) for a 10-minute benchmark:
25% means one core load

Task Configuration Time (s) CPU load
decode original 18.05 34%
decode 1thread 18.05 34%
decode 2threads 15.00 40%
decode 4threads 13.40 46%
decode 4th with 2nd commit 12.60 48%
encode original 27.50 25%
encode 4threads 15.55 49%
encode mergeZip 11.45 53%

Note: backsmali can't be properly multithreaded because of the synchronized methods inside
Now we're not compressing the same data twice
@iBotPeaches
Copy link
Owner

Interesting in timing how we got 1 PR doing ParallelStreams and this one doing an executor service. It what I was basically responding to in this other pr: #3411 (comment)

In short - thanks, let me review this.

Copy link
Owner

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

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

Took a quick static look. I have time this weekend to pull it down and do some testing.

@Cregrant
Copy link
Contributor Author

Another suggestion is to change the Gradle distribution from all to bin if you didn't choose all intentionally.

@iBotPeaches
Copy link
Owner

Another suggestion is to change the Gradle distribution from all to bin if you didn't choose all intentionally.

We did swap from bin -> all a few years ago for some reason. Forgot why.

Copy link
Owner

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

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

I ran some tests and I like it. I'll put in another PR shortly to:

  • Make jobs configurable
  • Explain how many jobs are running in info message at start of build
  • Remove unused method in ZipUtils

@Cregrant
Copy link
Contributor Author

Thanks! I thought about defining the thread count and found that setting the number in the Main class (or setting it to 1 if a debug flag is set) via a public field is the best solution. This is because you probably won't have any reason to change it in the middle of decompiling.

@iBotPeaches iBotPeaches changed the title Performance yet again Feature: Parallel Building Dec 26, 2023
@iBotPeaches iBotPeaches merged commit 81aae69 into iBotPeaches:master Dec 26, 2023
25 checks passed
@iBotPeaches iBotPeaches added this to the v2.10.0 milestone Dec 26, 2023
iBotPeaches added a commit that referenced this pull request Dec 26, 2023
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