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

Optimize Proguard rules #4291

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

cbeyls
Copy link
Contributor

@cbeyls cbeyls commented Feb 28, 2024

Using saner defaults for R8 while reducing the app size even further.

  • Add Kotlin compiler options to skip adding assertions in release builds
  • Remove optimizations, optimizationpasses and dontpreverify rules that are ignored by R8
  • Only keep runtime annotations by default. If other attributes are needed by a specific library, these will already be provided by the library rules (for example Retrofit or coroutines)
  • Remove the obsolete rule allowing a View to reflectively call any arbitrary public Activity method accepting a View as argument. This has always been a bad practice and is not used in this project anyway
  • Remove the rules related to enums. R8 already optimizes enums properly out-of-the-box and keeping these rules may prevent some of these optimizations
  • Add support for the @Keep annotation. Even if it's not currently used in the code base, it can be handy in the future
  • Add a missing rule to prevent generic signature of NetworkResult class from being removed in MastodonApi so Retrofit works
  • Allow obfuscation and shrinking of kotlin.coroutines.Continuation, matching the rule defined in the next release of Retrofit
  • Remove the rule forcing the removal of String.format(). This method is actually used in the code (and in third-party libraries) for other things than logging so forcing its removal can do more harm than good.

@connyduck
Copy link
Collaborator

connyduck commented Feb 28, 2024

Thx!

Add support for the @keep annotation. Even if it's not currently used in the code base, it can be handy in the future

not necessary, these rules are included in the annotation library

Add a missing rule to prevent generic signature of NetworkResult class from being removed in MastodonApi so Retrofit works

not necessary, that rule is included in the NetworkResult library

Remove the rule forcing the removal of String.format(). This method is actually used in the code (and in third-party libraries) for other things than logging so forcing its removal can do more harm than good.

I disagree, it will only be removed if the result is not used. Take this line as an example: The log statement is removed, but without the String.format rule a dangling String.format will remain in the bytecode and we don't want that.

…d restore the orphaned String.format() removal rule
@cbeyls
Copy link
Contributor Author

cbeyls commented Feb 28, 2024

Thx!

Add support for the @keep annotation. Even if it's not currently used in the code base, it can be handy in the future

not necessary, these rules are included in the annotation library

I didn't know that, thanks for the info. I'll remove this from my other projects as well.

Add a missing rule to prevent generic signature of NetworkResult class from being removed in MastodonApi so Retrofit works

not necessary, that rule is included in the NetworkResult library

I didn't think about checking the CallAdapter library rules first, indeed it makes sense to provide the rule with the library.

Remove the rule forcing the removal of String.format(). This method is actually used in the code (and in third-party libraries) for other things than logging so forcing its removal can do more harm than good.

I disagree, it will only be removed if the result is not used. Take this line as an example: The log statement is removed, but without the String.format rule a dangling String.format will remain in the bytecode and we don't want that.

Indeed, in the case of String.format() the method actually has no side effects so it's safe to keep the rule. I'll re-add it.

However, this also means that there are probably other kinds of String formatting code that end up orphaned after removing the Log calls.

Ideally the Log calls should be replaced with Kotlin inline functions taking a lamba argument (where all the formatting takes place) and checking a global boolean constant before logging, that's the only reliable way to remove all logging code and its associated formatting. But that's another topic entirely.

Copy link
Collaborator

@connyduck connyduck left a comment

Choose a reason for hiding this comment

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

This shaves about 200kB off the apk size, very nice.

@connyduck connyduck merged commit 722b75e into tuskyapp:develop Feb 29, 2024
3 checks passed
@cbeyls cbeyls deleted the chore/improved_proguard_rules branch February 29, 2024 19:46
connyduck added a commit that referenced this pull request Mar 4, 2024
…4299)

Regression from #4291 // cc @cbeyls 

<details>
  <summary>Stacktrace</summary>
  
  ```
Process: com.keylesspalace.tusky, PID: 31230
java.lang.RuntimeException: Unable to start activity
ComponentInfo{com.keylesspalace.tusky/com.keylesspalace.tusky.components.accountlist.AccountListActivity}:
java.lang.RuntimeException: java.lang.NoSuchMethodException: h4.a.values
[]
at
android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3635)
at
android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3792)
at
android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103)
at
android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
at
android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2210)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loopOnce(Looper.java:201)
	at android.os.Looper.loop(Looper.java:288)
	at android.app.ActivityThread.main(ActivityThread.java:7839)
	at java.lang.reflect.Method.invoke(Native Method)
at
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
Caused by: java.lang.RuntimeException: java.lang.NoSuchMethodException:
h4.a.values []
	at java.lang.Enum.enumValues(Enum.java:270)
	at java.lang.Enum.access$000(Enum.java:61)
	at java.lang.Enum$1.create(Enum.java:277)
	at java.lang.Enum$1.create(Enum.java:275)
	at libcore.util.BasicLruCache.get(BasicLruCache.java:63)
	at java.lang.Enum.getSharedConstants(Enum.java:289)
	at java.lang.Enum.valueOf(Enum.java:243)
	at java.io.ObjectInputStream.readEnum(ObjectInputStream.java:1841)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1409)
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:427)
	at android.os.Parcel.readSerializable(Parcel.java:3507)
	at android.os.Parcel.readValue(Parcel.java:3277)
	at android.os.Parcel.readArrayMapInternal(Parcel.java:3623)
at android.os.BaseBundle.initializeFromParcelLocked(BaseBundle.java:292)
	at android.os.BaseBundle.unparcel(BaseBundle.java:236)
	at android.os.BaseBundle.getSerializable(BaseBundle.java:1268)
	at android.os.Bundle.getSerializable(Bundle.java:1104)
	at android.content.Intent.getSerializableExtra(Intent.java:8575)
at
com.keylesspalace.tusky.components.accountlist.AccountListActivity.onCreate(SourceFile:23)
	at android.app.Activity.performCreate(Activity.java:8051)
	at android.app.Activity.performCreate(Activity.java:8031)
at
android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1329)
at
android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3608)
at
android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3792) 
at
android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103) 
at
android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135) 
at
android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95) 
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2210) 
	at android.os.Handler.dispatchMessage(Handler.java:106) 
	at android.os.Looper.loopOnce(Looper.java:201) 
	at android.os.Looper.loop(Looper.java:288) 
	at android.app.ActivityThread.main(ActivityThread.java:7839) 
	at java.lang.reflect.Method.invoke(Native Method) 
at
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) 
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) 
Caused by: java.lang.NoSuchMethodException: h4.a.values []
	at java.lang.Class.getMethod(Class.java:2103)
	at java.lang.Class.getDeclaredMethod(Class.java:2081)
	at java.lang.Enum.enumValues(Enum.java:267)
	at java.lang.Enum.access$000(Enum.java:61) 
	at java.lang.Enum$1.create(Enum.java:277) 
	at java.lang.Enum$1.create(Enum.java:275) 
	at libcore.util.BasicLruCache.get(BasicLruCache.java:63) 
	at java.lang.Enum.getSharedConstants(Enum.java:289) 
	at java.lang.Enum.valueOf(Enum.java:243) 
	at java.io.ObjectInputStream.readEnum(ObjectInputStream.java:1841) 
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1409) 
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:427) 
	at android.os.Parcel.readSerializable(Parcel.java:3507) 
	at android.os.Parcel.readValue(Parcel.java:3277) 
	at android.os.Parcel.readArrayMapInternal(Parcel.java:3623) 
at
android.os.BaseBundle.initializeFromParcelLocked(BaseBundle.java:292) 
	at android.os.BaseBundle.unparcel(BaseBundle.java:236) 
	at android.os.BaseBundle.getSerializable(BaseBundle.java:1268) 
	at android.os.Bundle.getSerializable(Bundle.java:1104) 
	at android.content.Intent.getSerializableExtra(Intent.java:8575) 
at
com.keylesspalace.tusky.components.accountlist.AccountListActivity.onCreate(SourceFile:23) 
	at android.app.Activity.performCreate(Activity.java:8051) 
	at android.app.Activity.performCreate(Activity.java:8031) 
at
android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1329) 
at
android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3608) 
at
android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3792) 
at
android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103) 
at
android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135) 
at
android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95) 
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2210) 
	at android.os.Handler.dispatchMessage(Handler.java:106) 
	at android.os.Looper.loopOnce(Looper.java:201) 
	at android.os.Looper.loop(Looper.java:288) 
	at android.app.ActivityThread.main(ActivityThread.java:7839) 
	at java.lang.reflect.Method.invoke(Native Method) 
at
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) 
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) 
  ```
</details>

closes #4297
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