-
Notifications
You must be signed in to change notification settings - Fork 411
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
base: master
Are you sure you want to change the base?
Changes from 33 commits
274fd19
043be3a
c098ad8
7922f11
bb66672
573995e
e4ce1e0
70e5966
aabc5bd
f4a22fa
7b2146e
3573c19
025571c
845d044
13b38d5
fb9fe7d
cf74760
2090514
fb5eedb
6ae5fc3
e90c5c9
33aa83d
1d49b49
cb60c13
3d3bf8f
8c354ab
813be34
f155107
7d82d2d
047719c
5cc6748
1f5c85b
54233a5
ef37241
beeadd5
343df22
71213fe
42493c6
8cfd505
cff6565
5644a9d
48c52e5
badf492
61e1e98
00664b2
fca2cde
f8f1155
bcab91e
2710ff3
49bd302
55b4f0e
7613999
ee3e3ab
f1ba773
7d602c9
4d18370
2648338
b431cb7
11f21f1
028e8c4
0cc53cd
a440b45
7e652b3
607bf86
f69fd14
95b23ab
c5bca4a
23d4efe
7cdac26
03ae5c6
ad8b5ff
26d9850
a5ebb5e
71b63a4
7da8e01
751cc74
f4f84e4
9d080b2
b8fbcd0
3a866fc
a910309
0cf13a7
1ba33ba
ceea1b6
93d3bb6
ac7f42d
370704b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package proguard.optimize.inline; | ||
|
||
public class CannotInlineException extends RuntimeException { | ||
public CannotInlineException(String reason) { | ||
super(reason); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package proguard.optimize.inline; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import proguard.classfile.Clazz; | ||
import proguard.classfile.Method; | ||
import proguard.classfile.attribute.CodeAttribute; | ||
import proguard.classfile.constant.Constant; | ||
import proguard.classfile.constant.MethodrefConstant; | ||
import proguard.classfile.constant.visitor.ConstantVisitor; | ||
import proguard.classfile.editor.CodeAttributeEditor; | ||
import proguard.classfile.editor.InstructionSequenceBuilder; | ||
import proguard.classfile.instruction.Instruction; | ||
import proguard.classfile.instruction.visitor.InstructionVisitor; | ||
import proguard.classfile.util.InstructionSequenceMatcher; | ||
import proguard.classfile.visitor.ClassPrinter; | ||
|
||
import java.io.PrintWriter; | ||
import java.io.StringWriter; | ||
|
||
public class CastPatternRemover implements InstructionVisitor { | ||
private final Logger logger = LogManager.getLogger(this.getClass()); | ||
private final InstructionSequenceMatcher insSeqMatcher; | ||
private final CodeAttributeEditor codeAttributeEditor; | ||
|
||
public CastPatternRemover(CodeAttributeEditor codeAttributeEditor) { | ||
InstructionSequenceBuilder ____ = new InstructionSequenceBuilder(); | ||
Constant[] constants = ____.constants(); | ||
Instruction[] pattern = ____.invokestatic(InstructionSequenceMatcher.X).areturn().__(); | ||
this.insSeqMatcher = new InstructionSequenceMatcher(constants, pattern); | ||
this.codeAttributeEditor = codeAttributeEditor; | ||
} | ||
|
||
@Override | ||
public void visitAnyInstruction(Clazz clazz, Method method, CodeAttribute codeAttribute, int offset, Instruction instruction) { | ||
instruction.accept(clazz, method, codeAttribute, offset, insSeqMatcher); | ||
if (insSeqMatcher.isMatching()) { | ||
int constantIndex = insSeqMatcher.matchedConstantIndex(InstructionSequenceMatcher.X); | ||
clazz.constantPoolEntryAccept(constantIndex, new ConstantVisitor() { | ||
@Override | ||
public void visitMethodrefConstant(Clazz clazz, MethodrefConstant methodrefConstant) { | ||
if (methodrefConstant.getName(clazz).equals("valueOf")) { | ||
if (logger.isDebugEnabled()) { | ||
StringWriter stringWriter = new StringWriter(); | ||
instruction.accept(clazz, method, codeAttribute, offset, new ClassPrinter(new PrintWriter(stringWriter))); | ||
logger.debug("Removing " + stringWriter); | ||
} | ||
|
||
codeAttributeEditor.deleteInstruction(insSeqMatcher.matchedInstructionOffset(0)); | ||
} | ||
} | ||
}); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package proguard.optimize.inline; | ||
|
||
import proguard.classfile.Clazz; | ||
import proguard.classfile.Method; | ||
import proguard.classfile.attribute.CodeAttribute; | ||
import proguard.classfile.constant.AnyMethodrefConstant; | ||
import proguard.classfile.constant.NameAndTypeConstant; | ||
import proguard.classfile.constant.visitor.ConstantVisitor; | ||
import proguard.classfile.editor.CodeAttributeEditor; | ||
import proguard.classfile.instruction.ConstantInstruction; | ||
import proguard.classfile.instruction.Instruction; | ||
import proguard.classfile.instruction.visitor.InstructionVisitor; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Set; | ||
|
||
public class CastRemover implements InstructionVisitor { | ||
TimothyGeerkensGuardsquare marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private final CodeAttributeEditor codeAttributeEditor; | ||
private final List<Integer> keepList; | ||
private int argIndex; | ||
|
||
// The names of all method taking a boxed type variable and returning the variable with the unboxed type | ||
MaartenS11 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private final Set<String> castingMethodNames = new HashSet<>(Arrays.asList("intValue", "booleanValue", "byteValue", "shortValue", "longValue", "floatValue", "doubleValue", "charValue")); | ||
|
||
|
||
public CastRemover(CodeAttributeEditor codeAttributeEditor, List<Integer> keepList) { | ||
this.codeAttributeEditor = codeAttributeEditor; | ||
this.argIndex = 0; | ||
this.keepList = keepList; | ||
} | ||
|
||
public CastRemover(CodeAttributeEditor codeAttributeEditor) { | ||
this(codeAttributeEditor, new ArrayList<>()); | ||
} | ||
|
||
@Override | ||
public void visitConstantInstruction(Clazz clazz, Method method, CodeAttribute codeAttribute, int offset, ConstantInstruction constantInstruction) { | ||
TimothyGeerkensGuardsquare marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/* Casting on a lambda invoke call looks like this : | ||
* iload | ||
* invokestatic valueOf | ||
* invokeinterface invoke //lambda call | ||
* checkast | ||
* invokevirtual intValue | ||
* istore | ||
* | ||
* We remove valueOf, checkast and intValue | ||
*/ | ||
if (constantInstruction.opcode == Instruction.OP_CHECKCAST) { | ||
codeAttributeEditor.deleteInstruction(offset); | ||
} else if (constantInstruction.opcode == Instruction.OP_INVOKESTATIC) { | ||
if (getInvokedMethodName(clazz, constantInstruction).equals("valueOf")) { | ||
// Don't remove valueOf call when the lambda takes an object as argument | ||
if (!keepList.contains(argIndex)) { | ||
codeAttributeEditor.deleteInstruction(offset); | ||
} | ||
argIndex++; | ||
} | ||
} else if (constantInstruction.opcode == Instruction.OP_INVOKEVIRTUAL) { | ||
if (castingMethodNames.contains(getInvokedMethodName(clazz, constantInstruction))) { | ||
codeAttributeEditor.deleteInstruction(offset); | ||
} | ||
} | ||
} | ||
|
||
@Override | ||
public void visitAnyInstruction(Clazz clazz, Method method, CodeAttribute codeAttribute, int offset, Instruction instruction) { | ||
} | ||
|
||
private String getInvokedMethodName(Clazz clazz, ConstantInstruction constantInstruction) { | ||
final String[] invokedMethodName = new String[1]; | ||
clazz.constantPoolEntryAccept(constantInstruction.constantIndex, new ConstantVisitor() { | ||
@Override public void visitAnyMethodrefConstant(Clazz clazz, AnyMethodrefConstant anyMethodrefConstant) { | ||
clazz.constantPoolEntryAccept(anyMethodrefConstant.u2nameAndTypeIndex, new ConstantVisitor() { | ||
@Override | ||
public void visitNameAndTypeConstant(Clazz clazz, NameAndTypeConstant nameAndTypeConstant) { | ||
invokedMethodName[0] = nameAndTypeConstant.getName(clazz); | ||
} | ||
}); | ||
} | ||
}); | ||
return invokedMethodName[0]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
package proguard.optimize.inline; | ||
|
||
import proguard.classfile.Clazz; | ||
import proguard.classfile.ProgramClass; | ||
import proguard.classfile.ProgramMethod; | ||
import proguard.classfile.editor.AttributeAdder; | ||
import proguard.classfile.editor.ClassBuilder; | ||
import proguard.classfile.editor.ClassEditor; | ||
import proguard.obfuscate.NameFactory; | ||
import proguard.obfuscate.SimpleNameFactory; | ||
import proguard.obfuscate.UniqueMemberNameFactory; | ||
import proguard.optimize.info.ProgramMemberOptimizationInfoSetter; | ||
|
||
import java.util.function.Function; | ||
|
||
public class DescriptorModifier { | ||
private final ClassBuilder classBuilder; | ||
private final NameFactory nameFactory; | ||
private final Clazz clazz; | ||
|
||
DescriptorModifier(Clazz clazz) { | ||
classBuilder = new ClassBuilder((ProgramClass) clazz); | ||
nameFactory = new UniqueMemberNameFactory(new SimpleNameFactory(), clazz); | ||
this.clazz = clazz; | ||
} | ||
|
||
public ProgramMethod modify(ProgramMethod sourceMethod, Function<String, String> modifier) { | ||
return modify(sourceMethod, modifier, false); | ||
} | ||
|
||
public ProgramMethod modify(ProgramMethod sourceMethod, Function<String, String> modifier, boolean removeOriginal) { | ||
ProgramMethod newMethod = classBuilder.addAndReturnMethod( | ||
sourceMethod.u2accessFlags, | ||
nameFactory.nextName(), | ||
modifier.apply(sourceMethod.getDescriptor(clazz)) | ||
); | ||
|
||
sourceMethod.attributesAccept((ProgramClass) clazz, | ||
new AttributeAdder((ProgramClass) clazz, newMethod, true)); | ||
|
||
if (removeOriginal) | ||
removeOriginal(sourceMethod); | ||
|
||
newMethod.accept(clazz, new ProgramMemberOptimizationInfoSetter()); | ||
return newMethod; | ||
} | ||
|
||
private void removeOriginal(ProgramMethod method) { | ||
ClassEditor classEditor = new ClassEditor((ProgramClass) clazz); | ||
classEditor.removeMethod(method); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package proguard.optimize.inline; | ||
|
||
import proguard.classfile.util.InternalTypeEnumeration; | ||
|
||
public class FirstLambdaParameterFinder { | ||
public static int findFirstLambdaParameter(String descriptor) { | ||
return findFirstLambdaParameter(descriptor, true); | ||
} | ||
|
||
public static int findFirstLambdaParameter(String descriptor, boolean isStatic) { | ||
InternalTypeEnumeration internalTypeEnumeration = new InternalTypeEnumeration(descriptor); | ||
int index = 0; | ||
while (internalTypeEnumeration.hasMoreTypes()) { | ||
if (internalTypeEnumeration.nextType().startsWith("Lkotlin/jvm/functions/Function")) { | ||
break; | ||
} | ||
index ++; | ||
} | ||
return index + (isStatic ? 0 : 1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package proguard.optimize.inline; | ||
|
||
import proguard.classfile.instruction.Instruction; | ||
|
||
import java.util.Objects; | ||
|
||
public final class InstructionAtOffset { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it not more safe to use the CodeAttribute and Instruction for this? Using existing data types, classes,... helps to make use of the existing infrastructure which makes a lot of stuff easier. The infrastructure helps by performing updates on the data structures and performing validity checks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that if we only store a CodeAttribute and an Instruction we can't get the correct offset of the instruction reliably. |
||
private final Instruction instruction; | ||
private final int offset; | ||
|
||
InstructionAtOffset(Instruction instruction, int offset) { | ||
this.instruction = instruction; | ||
this.offset = offset; | ||
} | ||
|
||
public Instruction instruction() { | ||
return instruction; | ||
} | ||
|
||
public int offset() { | ||
return offset; | ||
} | ||
} |
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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 codeHasChanged() { | ||
return changed; | ||
} | ||
|
||
public void setChanged(boolean changed) { | ||
this.changed = 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.
this should probably happen inside the optimize method instead, and run once per optimization pass