Lambda inlining optimization #353

TimothyGeerkensGuardsquare · 2023-08-03T09:46:53Z

Added lambda inlining for Kotlin lambdas passed directly as an argument or stored in a variable then passed as an argument. It currently cannot inline lambdas that are passed to a method and then used as an argument to a method called from the first method. We currently do not inline from final fields, nor from lambdas returned from methods. So we just handle the most basic cases for now.

Retrace
Only inline lambdas based on certain rules
Start to end example with multiple steps in bytecode
Replace aconst_null with getstatic call to the lambda class
Add enable/disable option for the lambda inlining

Co-authored-by: Maarten Steevens <[email protected]> Co-authored-by: Timothy Geerkens <[email protected]>

rubenpieters · 2023-08-04T09:27:52Z

Can you provide some timings for a few applications: how big the applications are and how much time this inlining pass takes?

MaartenS11 · 2023-08-04T09:35:49Z

So currently the only real world application we tested it on is the klox compiler, it contains many lambda usages so it seemed like a good test. By just inlining the more basic lambdas we are currently focusing on we were able to get a ~10% performance increase. So this is without running proguard on it so only lambda inlining. Inlining the lambdas took around 30s.

Benchmark 1: java -jar result.jar wc.lox -o Test.jar
  Time (mean ± σ):      1.831 s ±  0.107 s    [User: 6.152 s, System: 0.238 s]
  Range (min … max):    1.717 s …  2.025 s    10 runs
 
Benchmark 1: java -jar test-files/klox.jar wc.lox -o Test2.jar
  Time (mean ± σ):      2.012 s ±  0.173 s    [User: 6.715 s, System: 0.238 s]
  Range (min … max):    1.822 s …  2.345 s    10 runs

We will try do run it on some more real world applications including android apps.

We ran the inliner on a real world application and found a lot of issues, this commit aims to resolve those.

SwingsOberon · 2023-08-04T10:20:51Z

base/src/main/java/proguard/Configuration.java

+    /**
+     * Specifies whether lambdas should be inlined.
+     */
+    public boolean                      lambdaInlining                   = true;


I'm not sure whether we would like to have this enabled by default but rather have this as an opt-in functionality. For opt-in I believe it should be false here.

base/src/main/java/proguard/ProGuard.java

base/src/main/java/proguard/optimize/inline/lambda_locator/LambdaLocator.java

base/src/main/java/proguard/optimize/inline/NullCheckRemover.java

base/src/main/java/proguard/optimize/inline/RecursiveInliner.java

base/src/main/java/proguard/optimize/inline/Util.java

SwingsOberon · 2023-08-04T12:23:53Z

base/src/test/kotlin/proguard/lambdaInline/OneLambdaInArgsTest.kt

+            """.trimIndent()
+        )
+
+        // TODO: Currently broken due to a bug in the proguard-core test utils


Is this still the case, I thought it was fixed?

It is fixed but that version of proguard core is unreleased. This pr makes use of the latest released build of proguard core which is 9.0.10.

… klox-fixes

Made LambdaUsageHandler a class Put loadJar inside testUtil.kt and removed LambdaLocator.Util Added comments in CastRemover

We can do some of these changes because we don't modify the code in this version

…odHandler class

…anymore

… klox-fixes

Refactored LambdaImplementationVisitor visitProgramMethod

Cleaned commented code in NullCheckRemover Removed unused class Removed staircase in LocalUsageRemover

…terativeInstructionVisitor class

… klox-fixes

Fixes from inlining lambdas in the klox compiler

added a logger in lambda locator

…he arguments

… descriptor This is better because it fixes the case where a lambda is attempted to be inlined into a method that takes object as argument which is then later casted back into a lambda.

…javadoc to Lambda class to explain what each argument/field represents

+ added debug messages

… lambda-inlining

…ally changed

…he BaseLambdaInliner.inline() method

TimothyGeerkensGuardsquare · 2023-08-10T12:02:45Z

Can you provide some timings for a few applications: how big the applications are and how much time this inlining pass takes?

Current performance on a few android applications :

ANOTHERpass :

Inlining pass runtime = 4078ms
Number of lambdas found = 24
APK size before proguard = 5.3MB
APK size after proguard = 3.2MB

OneList :

Inlining pass runtime = 4651ms
Number of lambdas found = 31
APK size before proguard = 5.2MB
APK size after proguard = 3.0MB

Antimine :

Inlining pass runtime = 47651ms
Number of lambdas found = 158
APK size before proguard = 10.1MB
APK size after proguard = 5.0MB

…ass because the visitors are not part of the API of the BaseLambdaInliner

…mbda inlining without recompiling

…e so it matches the package name used in the tests and it also is just more informative, the MethodInliner for example was not in there

…tlin

MaartenS11 · 2023-08-11T12:24:39Z

We noticed that proguard-core and proguard both have a version of the LambdaExpressionCollector, I guess it was moved over into proguard-core at some point but never removed here? They are also in the same package which can cause Intelij to understandably get a little confused sometimes.

(Location can maybe be changed not sure exactly where we should put it, putting it in the official documentation seems incorrect this is not really something a proguard user should know it's more of a developer oriented piece of documentation.)

…ferences have to be updated, just the classes involved in the inlining process This should improve performance a bit.

This should also improve performance a bit.

…lassPool This should again improve performance of the pass a bit.

… method and lambda length

sonarcloud · 2023-08-13T11:53:59Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
20 Code Smells

No Coverage information
0.0% Duplication

Added lambda inlining

274fd19

Co-authored-by: Maarten Steevens <[email protected]> Co-authored-by: Timothy Geerkens <[email protected]>

Fixes from inlining lambdas in the klox compiler

043be3a

We ran the inliner on a real world application and found a lot of issues, this commit aims to resolve those.

SwingsOberon requested changes Aug 4, 2023

View reviewed changes

TimothyGeerkensGuardsquare and others added 25 commits August 4, 2023 17:05

Added test for retracer + addressed Oberon's comment from index 1 to 9

c098ad8

Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…

7922f11

… klox-fixes

Fixed regression from merge

bb66672

Removed some unused fields from the RecursiveInliner

573995e

Removed import .* in LambdaLocator

e4ce1e0

Made LambdaUsageHandler a class Put loadJar inside testUtil.kt and removed LambdaLocator.Util Added comments in CastRemover

Simplified the RecursiveInliner a bit

70e5966

We can do some of these changes because we don't modify the code in this version

Cleaned up RecursiveInliner by moving some code into a new CalledMeth…

aabc5bd

…odHandler class

Move some code into IndyLambdaImplVisitor class

f4a22fa

Remove argument from RecursiveInliner because it isn't really needed …

7b2146e

…anymore

Reduce line length a bit by putting index calculation in a variable

3573c19

Remove wildcard imports from RecursiveInliner

025571c

Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…

845d044

… klox-fixes

Removed visitor staircase in LambdaLocator

13b38d5

Refactored LambdaImplementationVisitor visitProgramMethod

Removed wildcards in import

fb9fe7d

Cleaned commented code in NullCheckRemover Removed unused class Removed staircase in LocalUsageRemover

Cleaned up the while loop in the LambdaUsageFinder by using the new I…

cf74760

…terativeInstructionVisitor class

Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…

2090514

… klox-fixes

Merge pull request #1 from MaartenS11/klox-fixes

fb5eedb

Fixes from inlining lambdas in the klox compiler

Removed unused class and method

6ae5fc3

added a logger in lambda locator

merge

e90c5c9

Add more comments to the RecursiveInliner

33aa83d

removed print and added logging

1d49b49

refactored test and added performance tests

cb60c13

Only stop inlining if we actually have the current lambda in one of t…

3d3bf8f

…he arguments

refactored util

8c354ab

Use argument index from LambdaUsageFinder instead of searching in the…

813be34

… descriptor This is better because it fixes the case where a lambda is attempted to be inlined into a method that takes object as argument which is then later casted back into a lambda.

MaartenS11 and others added 10 commits August 9, 2023 18:28

Renamed constantInstruction and offset to be more precise also added …

7e652b3

…javadoc to Lambda class to explain what each argument/field represents

Added more info about what "a lambda" really means

607bf86

Change some things to the FixedPointCodeAttributeVisitor API

f69fd14

invokespecial is now considered a potential source for the sourcetracer

95b23ab

+ added debug messages

merge

c5bca4a

Use library and program class pools instead of passing AppView

23d4efe

Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…

7cdac26

… lambda-inlining

Made one of the debug log messages info again because it was accident…

03ae5c6

…ally changed

Add large javadoc comment explaining all the steps taken when using t…

ad8b5ff

…he BaseLambdaInliner.inline() method

Added comment explaining why descriptor is changed to use unboxed types

26d9850

MaartenS11 added 6 commits August 10, 2023 14:30

Move a lot of code from the BaseLambdaInliner into a private inner cl…

a5ebb5e

…ass because the visitors are not part of the API of the BaseLambdaInliner

Fixed incorrect comment mentioning method that no longer exists

71b63a4

Disable replacing usages with aconst_null

7da8e01

Add -inlinekotlinlambdas command line option to enable and disable la…

751cc74

…mbda inlining without recompiling

Moved lambda inlining optimization from inline to lambdainline packag…

f4f84e4

…e so it matches the package name used in the tests and it also is just more informative, the MethodInliner for example was not in there

Add CannotInlineException message to debug log

9d080b2

MaartenS11 force-pushed the lambda-inlining branch from 3f62c9c to 9d080b2 Compare August 10, 2023 16:26

MaartenS11 added 2 commits August 11, 2023 12:27

Add ProGuard header to every class

b8fbcd0

Added detection for the second null check method for parameters in Ko…

3a866fc

…tlin

TimothyGeerkensGuardsquare marked this pull request as ready for review August 11, 2023 12:25

MaartenS11 added 7 commits August 11, 2023 14:49

Fix example and explain more about the cast removal/boxing/unboxing

0cf13a7

Mention a bit more which classes are used to do which operations

1ba33ba

Be more specific when using the ClassReferenceInitializer, not all re…

ceea1b6

…ferences have to be updated, just the classes involved in the inlining process This should improve performance a bit.

Only re-run the ClassReferenceInitializer on the consuming class

93d3bb6

This should also improve performance a bit.

Only run AccessFixer on consumingClass instead of the entire programC…

ac7f42d

…lassPool This should again improve performance of the pass a bit.

Adjust conditional inlining attempt condition to use sum of consuming…

370704b

… method and lambda length

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lambda inlining optimization #353

Lambda inlining optimization #353

TimothyGeerkensGuardsquare commented Aug 3, 2023 •

edited

Loading

rubenpieters commented Aug 4, 2023

MaartenS11 commented Aug 4, 2023

SwingsOberon Aug 4, 2023

SwingsOberon Aug 4, 2023

MaartenS11 Aug 5, 2023

TimothyGeerkensGuardsquare commented Aug 10, 2023

MaartenS11 commented Aug 11, 2023 •

edited

Loading

sonarcloud bot commented Aug 13, 2023

Lambda inlining optimization #353

Are you sure you want to change the base?

Lambda inlining optimization #353

Conversation

TimothyGeerkensGuardsquare commented Aug 3, 2023 • edited Loading

rubenpieters commented Aug 4, 2023

MaartenS11 commented Aug 4, 2023

SwingsOberon Aug 4, 2023

Choose a reason for hiding this comment

SwingsOberon Aug 4, 2023

Choose a reason for hiding this comment

MaartenS11 Aug 5, 2023

Choose a reason for hiding this comment

TimothyGeerkensGuardsquare commented Aug 10, 2023

MaartenS11 commented Aug 11, 2023 • edited Loading

sonarcloud bot commented Aug 13, 2023

TimothyGeerkensGuardsquare commented Aug 3, 2023 •

edited

Loading

MaartenS11 commented Aug 11, 2023 •

edited

Loading