-
Notifications
You must be signed in to change notification settings - Fork 421
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
Lambda inlining optimization #353
Open
TimothyGeerkensGuardsquare
wants to merge
87
commits into
Guardsquare:master
Choose a base branch
from
MaartenS11:lambda-inlining
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
87 commits
Select commit
Hold shift + click to select a range
274fd19
Added lambda inlining
TimothyGeerkensGuardsquare 043be3a
Fixes from inlining lambdas in the klox compiler
MaartenS11 c098ad8
Added test for retracer + addressed Oberon's comment from index 1 to 9
TimothyGeerkensGuardsquare 7922f11
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 bb66672
Fixed regression from merge
MaartenS11 573995e
Removed some unused fields from the RecursiveInliner
MaartenS11 e4ce1e0
Removed import .* in LambdaLocator
TimothyGeerkensGuardsquare 70e5966
Simplified the RecursiveInliner a bit
MaartenS11 aabc5bd
Cleaned up RecursiveInliner by moving some code into a new CalledMeth…
MaartenS11 f4a22fa
Move some code into IndyLambdaImplVisitor class
MaartenS11 7b2146e
Remove argument from RecursiveInliner because it isn't really needed …
MaartenS11 3573c19
Reduce line length a bit by putting index calculation in a variable
MaartenS11 025571c
Remove wildcard imports from RecursiveInliner
MaartenS11 845d044
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 13b38d5
Removed visitor staircase in LambdaLocator
TimothyGeerkensGuardsquare fb9fe7d
Removed wildcards in import
TimothyGeerkensGuardsquare cf74760
Cleaned up the while loop in the LambdaUsageFinder by using the new I…
MaartenS11 2090514
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 fb5eedb
Merge pull request #1 from MaartenS11/klox-fixes
MaartenS11 6ae5fc3
Removed unused class and method
TimothyGeerkensGuardsquare e90c5c9
merge
TimothyGeerkensGuardsquare 33aa83d
Add more comments to the RecursiveInliner
MaartenS11 1d49b49
removed print and added logging
TimothyGeerkensGuardsquare cb60c13
refactored test and added performance tests
TimothyGeerkensGuardsquare 3d3bf8f
Only stop inlining if we actually have the current lambda in one of t…
MaartenS11 8c354ab
refactored util
TimothyGeerkensGuardsquare 813be34
Use argument index from LambdaUsageFinder instead of searching in the…
MaartenS11 f155107
RecursiveInliner should also use the argument index from the lambda u…
MaartenS11 7d82d2d
Remove findFirstLambdaParameter method since it is no longer needed
MaartenS11 047719c
WIP commit (please amend)
MaartenS11 5cc6748
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 1f5c85b
Merge pull request #2 from MaartenS11/recursive-inliner-adjustments
MaartenS11 54233a5
Put stop condition that was duplicated into a method
MaartenS11 ef37241
Don't hardcode the MethodInliner max resulting code size
MaartenS11 beeadd5
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 343df22
Fixed cast removal so it works in all cases
MaartenS11 71213fe
Reverted some minor accidental changes
MaartenS11 42493c6
Cleaned up a bit
MaartenS11 8cfd505
Merge pull request #3 from MaartenS11/more-fixes
MaartenS11 cff6565
Conditionally inline lambdas
MaartenS11 5644a9d
Added logging when a lambda has been inlined and only print method le…
MaartenS11 48c52e5
Log the consuming method and invoke method so the use knows which met…
MaartenS11 badf492
Merge pull request #4 from MaartenS11/inline-conditionally
MaartenS11 61e1e98
Fixed typo in comment
MaartenS11 00664b2
Removed some unnecessary comments from the tests
MaartenS11 fca2cde
Fixed typo in comment
MaartenS11 f8f1155
Rename package to lambdainline instead of lambdaInline
MaartenS11 bcab91e
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 2710ff3
Rename lambda_locator package to lambdalocator
MaartenS11 49bd302
Added newline to Lambda class and removed some unneeded methods
MaartenS11 55b4f0e
Use result of lambdaUsageHandler.handle() directly instead of storing…
MaartenS11 7613999
This doesn't have to be a nested if
MaartenS11 ee3e3ab
Added more javadoc comments to the IterativeInstructionVisitor
MaartenS11 f1ba773
Added more javadoc strings and fixed a typo
MaartenS11 7d602c9
Renamed isSourceInstruction to isNotSourceInstruction
TimothyGeerkensGuardsquare 4d18370
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
TimothyGeerkensGuardsquare 2648338
Added more javadoc comments
MaartenS11 b431cb7
Apply suggestions from code review
MaartenS11 11f21f1
Use Optional in MethodLengthFinder so the programmer is made aware th…
MaartenS11 028e8c4
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 0cc53cd
Return ProgramMethod instead of just Method
MaartenS11 a440b45
Use the name kotlin lambdas instead of static lambdas in the LambdaLo…
MaartenS11 7e652b3
Renamed constantInstruction and offset to be more precise also added …
MaartenS11 607bf86
Added more info about what "a lambda" really means
MaartenS11 f69fd14
Change some things to the FixedPointCodeAttributeVisitor API
MaartenS11 95b23ab
invokespecial is now considered a potential source for the sourcetracer
TimothyGeerkensGuardsquare c5bca4a
merge
TimothyGeerkensGuardsquare 23d4efe
Use library and program class pools instead of passing AppView
MaartenS11 7cdac26
Merge branch 'lambda-inlining' of github.com:MaartenS11/proguard into…
MaartenS11 03ae5c6
Made one of the debug log messages info again because it was accident…
MaartenS11 ad8b5ff
Add large javadoc comment explaining all the steps taken when using t…
MaartenS11 26d9850
Added comment explaining why descriptor is changed to use unboxed types
MaartenS11 a5ebb5e
Move a lot of code from the BaseLambdaInliner into a private inner cl…
MaartenS11 71b63a4
Fixed incorrect comment mentioning method that no longer exists
MaartenS11 7da8e01
Disable replacing usages with aconst_null
MaartenS11 751cc74
Add -inlinekotlinlambdas command line option to enable and disable la…
MaartenS11 f4f84e4
Moved lambda inlining optimization from inline to lambdainline packag…
MaartenS11 9d080b2
Add CannotInlineException message to debug log
MaartenS11 b8fbcd0
Add ProGuard header to every class
MaartenS11 3a866fc
Added detection for the second null check method for parameters in Ko…
MaartenS11 a910309
Added WIP README file to the lambdainline package
MaartenS11 0cf13a7
Fix example and explain more about the cast removal/boxing/unboxing
MaartenS11 1ba33ba
Mention a bit more which classes are used to do which operations
MaartenS11 ceea1b6
Be more specific when using the ClassReferenceInitializer, not all re…
MaartenS11 93d3bb6
Only re-run the ClassReferenceInitializer on the consuming class
MaartenS11 ac7f42d
Only run AccessFixer on consumingClass instead of the entire programC…
MaartenS11 370704b
Adjust conditional inlining attempt condition to use sum of consuming…
MaartenS11 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Cleaned up the while loop in the LambdaUsageFinder by using the new I…
…terativeInstructionVisitor class
- Loading branch information
commit cf74760194259f6fc62084772d6ad6d3fe057ced
There are no files selected for viewing
39 changes: 39 additions & 0 deletions
39
base/src/main/java/proguard/optimize/inline/IterativeInstructionVisitor.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package proguard.optimize.inline; | ||
|
||
import proguard.classfile.Clazz; | ||
import proguard.classfile.Method; | ||
import proguard.classfile.attribute.CodeAttribute; | ||
import proguard.classfile.instruction.Instruction; | ||
import proguard.classfile.instruction.InstructionFactory; | ||
import proguard.classfile.instruction.visitor.InstructionVisitor; | ||
|
||
public class IterativeInstructionVisitor { | ||
private boolean changed; | ||
|
||
/** | ||
* This helper method does something similar to codeAttribute.instructionsAccept but has the ability to stop and | ||
* restart visiting the instructions, this is useful for when you change the code during iteration. | ||
*/ | ||
public void instructionsAccept(Clazz clazz, Method method, CodeAttribute codeAttribute, InstructionVisitor instructionVisitor) { | ||
int offset = 0; | ||
while (offset < codeAttribute.u4codeLength) { | ||
Instruction instruction = InstructionFactory.create(codeAttribute.code, offset); | ||
int instructionLength = instruction.length(offset); | ||
changed = false; | ||
instruction.accept(clazz, method, codeAttribute, offset, instructionVisitor); | ||
if (changed) { | ||
offset = 0; | ||
continue; | ||
} | ||
offset += instructionLength; | ||
} | ||
} | ||
|
||
public boolean isChanged() { | ||
return changed; | ||
} | ||
|
||
public void setChanged(boolean changed) { | ||
this.changed = changed; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe/show a case where this class is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lambda usage finder looks at the program and tries to identify lambda usages, when it finds a lambda usage it will call the usage handler and it will inline the lambda. Because inlining the lambda results in a method call being replaced by a call to another method which has the lambda inlined and we could still inline lambdas into that new method because the method has multiple lambda arguments we need to stop the current visit and visit the new changed code attribute instead. This class provides a simple API to do exactly that, iterate over the instructions and restart the iteration once the code has changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see the need for this when you have nested lambdas. But I don't understand why it's necessary when you have multiple separate lambdas in one method. Why can't you register all the changes that need to be done and apply them with the
CodeAttributeEditor
in one go?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lambda inliner inlines one lambda at a time so you might have a call instruction to a method, the method has for example 2 lambdas as arguments. If we inline the first lambda we create a new method that now only has one argument. If we want to inline the second lambda we have to inline that lambda into the newly created method to get a new method that now has 0 arguments. We revisit the code attribute because when finding the usages we want to find the new method so we inline into the new method. If we don't do that you will get one method that takes in lambda 1 but not lambda 2 and one method that takes in lambda 2 but not lambda 1.