Revert "Refactor position remapper to better facilitate concurrency"
This reverts commit d466ac98e66916a238c57ccbb2da6e69ee834ceb.
Reason for revert: Nondeterministic mapping file
Bug: b/422947619
Change-Id: Ib6b16df9829505be5d85f0f5b0addb93414e4b95
diff --git a/src/main/java/com/android/tools/r8/debuginfo/DebugRepresentation.java b/src/main/java/com/android/tools/r8/debuginfo/DebugRepresentation.java
index e5db358..ed04432 100644
--- a/src/main/java/com/android/tools/r8/debuginfo/DebugRepresentation.java
+++ b/src/main/java/com/android/tools/r8/debuginfo/DebugRepresentation.java
@@ -475,13 +475,13 @@
}
public static DexInstruction getLastExecutableInstruction(DexInstruction[] instructions) {
- for (int i = instructions.length - 1; i >= 0; i--) {
- DexInstruction instruction = instructions[i];
+ DexInstruction lastInstruction = null;
+ for (DexInstruction instruction : instructions) {
if (!instruction.isPayload()) {
- return instruction;
+ lastInstruction = instruction;
}
}
- return null;
+ return lastInstruction;
}
private static int estimatedDebugInfoSize(DexDebugInfo info) {
diff --git a/src/main/java/com/android/tools/r8/utils/positions/AppPositionRemapper.java b/src/main/java/com/android/tools/r8/utils/positions/AppPositionRemapper.java
deleted file mode 100644
index 221ed96..0000000
--- a/src/main/java/com/android/tools/r8/utils/positions/AppPositionRemapper.java
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (c) 2025, the R8 project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-package com.android.tools.r8.utils.positions;
-
-import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.utils.CfLineToMethodMapper;
-import com.android.tools.r8.utils.positions.ClassPositionRemapper.IdentityPositionRemapper;
-import com.android.tools.r8.utils.positions.ClassPositionRemapper.KotlinInlineFunctionAppPositionRemapper;
-import com.android.tools.r8.utils.positions.ClassPositionRemapper.OptimizingPositionRemapper;
-
-public interface AppPositionRemapper {
-
- ClassPositionRemapper createClassPositionRemapper(DexProgramClass clazz);
-
- static AppPositionRemapper create(AppView<?> appView, CfLineToMethodMapper cfLineToMethodMapper) {
- boolean identityMapping = appView.options().lineNumberOptimization.isOff();
- AppPositionRemapper positionRemapper =
- identityMapping
- ? new IdentityPositionRemapper()
- : new OptimizingPositionRemapper(appView.options());
-
- // Kotlin inline functions and arguments have their inlining information stored in the
- // source debug extension annotation. Instantiate the kotlin remapper on top of the original
- // remapper to allow for remapping original positions to kotlin inline positions.
- return new KotlinInlineFunctionAppPositionRemapper(
- appView, positionRemapper, cfLineToMethodMapper);
- }
-}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/ClassFilePositionToMappedRangeMapper.java b/src/main/java/com/android/tools/r8/utils/positions/ClassFilePositionToMappedRangeMapper.java
index ef6d075..99c3df9 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/ClassFilePositionToMappedRangeMapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/ClassFilePositionToMappedRangeMapper.java
@@ -16,7 +16,6 @@
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Position.SyntheticPosition;
import com.android.tools.r8.utils.Pair;
-import com.android.tools.r8.utils.timing.Timing;
import java.util.ArrayList;
import java.util.List;
@@ -31,11 +30,10 @@
@Override
public List<MappedPosition> getMappedPositions(
ProgramMethod method,
- MethodPositionRemapper positionRemapper,
+ ClassPositionRemapper positionRemapper,
boolean hasOverloads,
boolean canUseDexPc,
- int pcEncodingCutoff,
- Timing timing) {
+ int pcEncodingCutoff) {
return appView.options().getTestingOptions().usePcEncodingInCfForTesting
? getPcEncodedPositions(method, positionRemapper)
: getMappedPositionsRemapped(method, positionRemapper, hasOverloads);
@@ -47,7 +45,7 @@
}
private List<MappedPosition> getMappedPositionsRemapped(
- ProgramMethod method, MethodPositionRemapper positionRemapper, boolean hasOverloads) {
+ ProgramMethod method, ClassPositionRemapper positionRemapper, boolean hasOverloads) {
List<MappedPosition> mappedPositions = new ArrayList<>();
// Do the actual processing for each method.
CfCode oldCode = method.getDefinition().getCode().asCfCode();
@@ -101,18 +99,21 @@
return mappedPositions;
}
+ @SuppressWarnings("UnusedVariable")
private List<MappedPosition> getPcEncodedPositions(
- ProgramMethod method, MethodPositionRemapper positionRemapper) {
+ ProgramMethod method, ClassPositionRemapper positionRemapper) {
List<MappedPosition> mappedPositions = new ArrayList<>();
// Do the actual processing for each method.
CfCode oldCode = method.getDefinition().getCode().asCfCode();
List<CfInstruction> oldInstructions = oldCode.getInstructions();
List<CfInstruction> newInstructions = new ArrayList<>(oldInstructions.size() * 3);
Position currentPosition = null;
+ boolean isFirstEntry = false;
for (CfInstruction oldInstruction : oldInstructions) {
if (oldInstruction.isPosition()) {
CfPosition cfPosition = oldInstruction.asPosition();
currentPosition = cfPosition.getPosition();
+ isFirstEntry = true;
} else {
if (currentPosition != null) {
Pair<Position, Position> remappedPosition =
@@ -124,6 +125,7 @@
newInstructions.add(position);
newInstructions.add(position.getLabel());
}
+ isFirstEntry = false;
newInstructions.add(oldInstruction);
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/ClassPositionRemapper.java b/src/main/java/com/android/tools/r8/utils/positions/ClassPositionRemapper.java
index f53d3f9..5444395 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/ClassPositionRemapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/ClassPositionRemapper.java
@@ -5,13 +5,13 @@
import com.android.tools.r8.ResourceException;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
-import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexValue.DexValueString;
-import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Position.SourcePosition;
import com.android.tools.r8.kotlin.KotlinSourceDebugExtensionParser;
@@ -21,7 +21,6 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Pair;
import java.util.IdentityHashMap;
-import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -29,20 +28,26 @@
// DexDebugPositionState) and returns a remapped Position.
public interface ClassPositionRemapper {
- MethodPositionRemapper createMethodPositionRemapper(List<ProgramMethod> methods);
+ Pair<Position, Position> createRemappedPosition(Position position);
- class IdentityPositionRemapper
- implements AppPositionRemapper, ClassPositionRemapper, MethodPositionRemapper {
+ static ClassPositionRemapper getPositionRemapper(
+ AppView<?> appView, CfLineToMethodMapper cfLineToMethodMapper) {
+ boolean identityMapping = appView.options().lineNumberOptimization.isOff();
+ ClassPositionRemapper positionRemapper =
+ identityMapping
+ ? new IdentityPositionRemapper()
+ : new OptimizingPositionRemapper(appView.options());
- @Override
- public ClassPositionRemapper createClassPositionRemapper(DexProgramClass clazz) {
- return this;
- }
+ // Kotlin inline functions and arguments have their inlining information stored in the
+ // source debug extension annotation. Instantiate the kotlin remapper on top of the original
+ // remapper to allow for remapping original positions to kotlin inline positions.
+ return new KotlinInlineFunctionPositionRemapper(
+ appView, positionRemapper, cfLineToMethodMapper);
+ }
- @Override
- public MethodPositionRemapper createMethodPositionRemapper(List<ProgramMethod> methods) {
- return this;
- }
+ void setCurrentMethod(DexEncodedMethod definition);
+
+ class IdentityPositionRemapper implements ClassPositionRemapper {
@Override
public Pair<Position, Position> createRemappedPosition(Position position) {
@@ -50,193 +55,157 @@
assert position.getOutlineCallee() == null;
return new Pair<>(position, position);
}
+
+ @Override
+ public void setCurrentMethod(DexEncodedMethod definition) {
+ // This has no effect.
+ }
}
- class OptimizingPositionRemapper implements AppPositionRemapper, ClassPositionRemapper {
-
+ class OptimizingPositionRemapper implements ClassPositionRemapper {
private final int maxLineDelta;
+ private DexMethod previousMethod = null;
+ private int previousSourceLine = -1;
+ private int nextOptimizedLineNumber = 1;
OptimizingPositionRemapper(InternalOptions options) {
- // TODO(b/113198295): For dex using "Constants.DBG_LINE_RANGE + Constants.DBG_LINE_BASE"
- // instead of 1 creates a ~30% smaller map file but the dex files gets larger due to reduced
- // debug info canonicalization.
+ // TODO(113198295): For dex using "Constants.DBG_LINE_RANGE + Constants.DBG_LINE_BASE"
+ // instead of 1 creates a ~30% smaller map file but the dex files gets larger due to reduced
+ // debug info canonicalization.
maxLineDelta = options.isGeneratingClassFiles() ? Integer.MAX_VALUE : 1;
}
@Override
- public ClassPositionRemapper createClassPositionRemapper(DexProgramClass clazz) {
- return this;
+ public Pair<Position, Position> createRemappedPosition(Position position) {
+ assert position.getMethod() != null;
+ if (position.getMethod().isIdenticalTo(previousMethod)) {
+ assert previousSourceLine >= 0;
+ if (position.getLine() > previousSourceLine
+ && position.getLine() - previousSourceLine <= maxLineDelta) {
+ nextOptimizedLineNumber += (position.getLine() - previousSourceLine) - 1;
+ }
+ }
+
+ Position newPosition =
+ position
+ .builderWithCopy()
+ .setLine(nextOptimizedLineNumber++)
+ .setCallerPosition(null)
+ .build();
+ previousSourceLine = position.getLine();
+ previousMethod = position.getMethod();
+ return new Pair<>(position, newPosition);
}
@Override
- public MethodPositionRemapper createMethodPositionRemapper(List<ProgramMethod> methods) {
- return new OptimizingMethodPositionRemapper();
- }
-
- class OptimizingMethodPositionRemapper implements MethodPositionRemapper {
-
- private DexMethod previousMethod = null;
- private int previousSourceLine = -1;
- private int nextOptimizedLineNumber = 1;
-
- @Override
- public Pair<Position, Position> createRemappedPosition(Position position) {
- assert position.getMethod() != null;
- if (position.getMethod().isIdenticalTo(previousMethod)) {
- assert previousSourceLine >= 0;
- if (position.getLine() > previousSourceLine
- && position.getLine() - previousSourceLine <= maxLineDelta) {
- nextOptimizedLineNumber += (position.getLine() - previousSourceLine) - 1;
- }
- }
-
- Position newPosition =
- position
- .builderWithCopy()
- .setLine(nextOptimizedLineNumber++)
- .setCallerPosition(null)
- .build();
- previousSourceLine = position.getLine();
- previousMethod = position.getMethod();
- return new Pair<>(position, newPosition);
- }
+ public void setCurrentMethod(DexEncodedMethod definition) {
+ // This has no effect.
}
}
- class KotlinInlineFunctionAppPositionRemapper implements AppPositionRemapper {
+ class KotlinInlineFunctionPositionRemapper implements ClassPositionRemapper {
private final AppView<?> appView;
- private final AppPositionRemapper baseRemapper;
private final DexItemFactory factory;
- private final CfLineToMethodMapper lineToMethodMapper;
-
private final Map<DexType, Result> parsedKotlinSourceDebugExtensions = new IdentityHashMap<>();
+ private final CfLineToMethodMapper lineToMethodMapper;
+ private final ClassPositionRemapper baseRemapper;
- KotlinInlineFunctionAppPositionRemapper(
+ // Fields for the current context.
+ private DexEncodedMethod currentMethod;
+ private Result parsedData = null;
+
+ private KotlinInlineFunctionPositionRemapper(
AppView<?> appView,
- AppPositionRemapper baseRemapper,
+ ClassPositionRemapper baseRemapper,
CfLineToMethodMapper lineToMethodMapper) {
this.appView = appView;
- this.baseRemapper = baseRemapper;
this.factory = appView.dexItemFactory();
+ this.baseRemapper = baseRemapper;
this.lineToMethodMapper = lineToMethodMapper;
}
@Override
- public ClassPositionRemapper createClassPositionRemapper(DexProgramClass clazz) {
- ClassPositionRemapper baseClassRemapper = baseRemapper.createClassPositionRemapper(clazz);
- assert baseClassRemapper == baseRemapper;
- return new KotlinInlineFunctionClassPositionRemapper(baseClassRemapper);
- }
-
- class KotlinInlineFunctionClassPositionRemapper implements ClassPositionRemapper {
-
- private final ClassPositionRemapper baseRemapper;
-
- private KotlinInlineFunctionClassPositionRemapper(ClassPositionRemapper baseRemapper) {
- this.baseRemapper = baseRemapper;
+ public Pair<Position, Position> createRemappedPosition(Position position) {
+ assert currentMethod != null;
+ int line = position.getLine();
+ Result parsedData = getAndParseSourceDebugExtension(position.getMethod().holder);
+ if (parsedData == null) {
+ return baseRemapper.createRemappedPosition(position);
}
-
- @Override
- public MethodPositionRemapper createMethodPositionRemapper(List<ProgramMethod> methods) {
- MethodPositionRemapper baseMethodRemapper =
- baseRemapper.createMethodPositionRemapper(methods);
- return new KotlinInlineFunctionMethodPositionRemapper(baseMethodRemapper, methods);
+ Map.Entry<Integer, KotlinSourceDebugExtensionParser.Position> inlinedPosition =
+ parsedData.lookupInlinedPosition(line);
+ if (inlinedPosition == null) {
+ return baseRemapper.createRemappedPosition(position);
}
-
- class KotlinInlineFunctionMethodPositionRemapper implements MethodPositionRemapper {
-
- private final MethodPositionRemapper baseRemapper;
- private final DexProgramClass currentHolder;
-
- private Result parsedData;
-
- private KotlinInlineFunctionMethodPositionRemapper(
- MethodPositionRemapper baseRemapper, List<ProgramMethod> methods) {
- assert !methods.isEmpty();
- this.baseRemapper = baseRemapper;
- this.currentHolder = methods.iterator().next().getHolder();
- }
-
- @Override
- public Pair<Position, Position> createRemappedPosition(Position position) {
- int line = position.getLine();
- Result parsedData = getAndParseSourceDebugExtension(position.getMethod().holder);
- if (parsedData == null) {
- return baseRemapper.createRemappedPosition(position);
- }
- Map.Entry<Integer, KotlinSourceDebugExtensionParser.Position> inlinedPosition =
- parsedData.lookupInlinedPosition(line);
- if (inlinedPosition == null) {
- return baseRemapper.createRemappedPosition(position);
- }
- int inlineeLineDelta = line - inlinedPosition.getKey();
- int originalInlineeLine = inlinedPosition.getValue().getRange().from + inlineeLineDelta;
- try {
- String binaryName = inlinedPosition.getValue().getSource().getPath();
- String nameAndDescriptor =
- lineToMethodMapper.lookupNameAndDescriptor(binaryName, originalInlineeLine);
- if (nameAndDescriptor == null) {
- return baseRemapper.createRemappedPosition(position);
- }
- String clazzDescriptor = DescriptorUtils.getDescriptorFromClassBinaryName(binaryName);
- String methodName = CfLineToMethodMapper.getName(nameAndDescriptor);
- String methodDescriptor = CfLineToMethodMapper.getDescriptor(nameAndDescriptor);
- String returnTypeDescriptor = DescriptorUtils.getReturnTypeDescriptor(methodDescriptor);
- String[] argumentDescriptors =
- DescriptorUtils.getArgumentTypeDescriptors(methodDescriptor);
- DexString[] argumentDexStringDescriptors = new DexString[argumentDescriptors.length];
- for (int i = 0; i < argumentDescriptors.length; i++) {
- argumentDexStringDescriptors[i] = factory.createString(argumentDescriptors[i]);
- }
- DexMethod inlinee =
- factory.createMethod(
- factory.createString(clazzDescriptor),
- factory.createString(methodName),
- factory.createString(returnTypeDescriptor),
- argumentDexStringDescriptors);
- if (!inlinee.equals(position.getMethod())) {
- // We have an inline from a different method than the current position.
- Entry<Integer, KotlinSourceDebugExtensionParser.Position> calleePosition =
- parsedData.lookupCalleePosition(line);
- if (calleePosition != null) {
- // Take the first line as the callee position
- int calleeLine = Math.max(0, calleePosition.getValue().getRange().from);
- position = position.builderWithCopy().setLine(calleeLine).build();
- }
- return baseRemapper.createRemappedPosition(
- SourcePosition.builder()
- .setLine(originalInlineeLine)
- .setMethod(inlinee)
- .setCallerPosition(position)
- .build());
- }
- // This is the same position, so we should really not mark this as an inline position.
- // Fall through to the default case.
- } catch (ResourceException ignored) {
- // Intentionally left empty. Remapping of kotlin functions utility is a best effort
- // mapping.
- }
+ int inlineeLineDelta = line - inlinedPosition.getKey();
+ int originalInlineeLine = inlinedPosition.getValue().getRange().from + inlineeLineDelta;
+ try {
+ String binaryName = inlinedPosition.getValue().getSource().getPath();
+ String nameAndDescriptor =
+ lineToMethodMapper.lookupNameAndDescriptor(binaryName, originalInlineeLine);
+ if (nameAndDescriptor == null) {
return baseRemapper.createRemappedPosition(position);
}
-
- private Result getAndParseSourceDebugExtension(DexType holder) {
- if (parsedData == null) {
- parsedData = parsedKotlinSourceDebugExtensions.get(holder);
- }
- if (parsedData != null || parsedKotlinSourceDebugExtensions.containsKey(holder)) {
- return parsedData;
- }
- DexValueString sourceDebugExtension =
- appView.getSourceDebugExtensionForType(currentHolder);
- if (sourceDebugExtension != null) {
- parsedData =
- KotlinSourceDebugExtensionParser.parse(sourceDebugExtension.getValue().toString());
- }
- parsedKotlinSourceDebugExtensions.put(holder, parsedData);
- return parsedData;
+ String clazzDescriptor = DescriptorUtils.getDescriptorFromClassBinaryName(binaryName);
+ String methodName = CfLineToMethodMapper.getName(nameAndDescriptor);
+ String methodDescriptor = CfLineToMethodMapper.getDescriptor(nameAndDescriptor);
+ String returnTypeDescriptor = DescriptorUtils.getReturnTypeDescriptor(methodDescriptor);
+ String[] argumentDescriptors = DescriptorUtils.getArgumentTypeDescriptors(methodDescriptor);
+ DexString[] argumentDexStringDescriptors = new DexString[argumentDescriptors.length];
+ for (int i = 0; i < argumentDescriptors.length; i++) {
+ argumentDexStringDescriptors[i] = factory.createString(argumentDescriptors[i]);
}
+ DexMethod inlinee =
+ factory.createMethod(
+ factory.createString(clazzDescriptor),
+ factory.createString(methodName),
+ factory.createString(returnTypeDescriptor),
+ argumentDexStringDescriptors);
+ if (!inlinee.equals(position.getMethod())) {
+ // We have an inline from a different method than the current position.
+ Entry<Integer, KotlinSourceDebugExtensionParser.Position> calleePosition =
+ parsedData.lookupCalleePosition(line);
+ if (calleePosition != null) {
+ // Take the first line as the callee position
+ int calleeLine = Math.max(0, calleePosition.getValue().getRange().from);
+ position = position.builderWithCopy().setLine(calleeLine).build();
+ }
+ return baseRemapper.createRemappedPosition(
+ SourcePosition.builder()
+ .setLine(originalInlineeLine)
+ .setMethod(inlinee)
+ .setCallerPosition(position)
+ .build());
+ }
+ // This is the same position, so we should really not mark this as an inline position. Fall
+ // through to the default case.
+ } catch (ResourceException ignored) {
+ // Intentionally left empty. Remapping of kotlin functions utility is a best effort mapping.
}
+ return baseRemapper.createRemappedPosition(position);
+ }
+
+ private Result getAndParseSourceDebugExtension(DexType holder) {
+ if (parsedData == null) {
+ parsedData = parsedKotlinSourceDebugExtensions.get(holder);
+ }
+ if (parsedData != null || parsedKotlinSourceDebugExtensions.containsKey(holder)) {
+ return parsedData;
+ }
+ DexClass clazz = appView.definitionFor(currentMethod.getHolderType());
+ DexValueString dexValueString = appView.getSourceDebugExtensionForType(clazz);
+ if (dexValueString != null) {
+ parsedData = KotlinSourceDebugExtensionParser.parse(dexValueString.value.toString());
+ }
+ parsedKotlinSourceDebugExtensions.put(holder, parsedData);
+ return parsedData;
+ }
+
+ @Override
+ public void setCurrentMethod(DexEncodedMethod method) {
+ this.currentMethod = method;
+ this.parsedData = null;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java b/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java
index 9430e76..0b28e8e 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java
@@ -26,7 +26,6 @@
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Position.SourcePosition;
-import com.android.tools.r8.utils.timing.Timing;
import java.util.ArrayList;
import java.util.List;
@@ -85,7 +84,7 @@
private final PositionEventEmitter positionEventEmitter;
private final List<MappedPosition> mappedPositions;
- private final MethodPositionRemapper positionRemapper;
+ private final ClassPositionRemapper positionRemapper;
private final List<DexDebugEvent> processedEvents;
// Keep track of what PC has been emitted.
@@ -96,7 +95,7 @@
public DexDebugPositionStateVisitor(
PositionEventEmitter positionEventEmitter,
List<MappedPosition> mappedPositions,
- MethodPositionRemapper positionRemapper,
+ ClassPositionRemapper positionRemapper,
List<DexDebugEvent> processedEvents,
DexItemFactory factory,
int startLine,
@@ -182,8 +181,7 @@
}
public List<MappedPosition> optimizeDexCodePositions(
- ProgramMethod method, MethodPositionRemapper positionRemapper, Timing timing) {
- timing.begin("No pc mapper");
+ ProgramMethod method, ClassPositionRemapper positionRemapper) {
List<MappedPosition> mappedPositions = new ArrayList<>();
// Do the actual processing for each method.
DexApplication application = appView.appInfo().app();
@@ -223,7 +221,6 @@
|| verifyIdentityMapping(method, debugInfo, optimizedDebugInfo);
dexCode.setDebugInfo(optimizedDebugInfo);
- timing.end();
return mappedPositions;
}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/DexPositionToPcMappedRangeMapper.java b/src/main/java/com/android/tools/r8/utils/positions/DexPositionToPcMappedRangeMapper.java
index 760f493..7b67169 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/DexPositionToPcMappedRangeMapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/DexPositionToPcMappedRangeMapper.java
@@ -19,7 +19,6 @@
import com.android.tools.r8.utils.IntBox;
import com.android.tools.r8.utils.Pair;
import com.android.tools.r8.utils.positions.PositionToMappedRangeMapper.PcBasedDebugInfoRecorder;
-import com.android.tools.r8.utils.timing.Timing;
import java.util.ArrayList;
import java.util.List;
@@ -35,18 +34,12 @@
}
public List<MappedPosition> optimizeDexCodePositionsForPc(
- ProgramMethod method,
- MethodPositionRemapper positionRemapper,
- int pcEncodingCutoff,
- Timing timing) {
- timing.begin("Pc mapper");
+ ProgramMethod method, ClassPositionRemapper positionRemapper, int pcEncodingCutoff) {
List<MappedPosition> mappedPositions = new ArrayList<>();
// Do the actual processing for each method.
DexCode dexCode = method.getDefinition().getCode().asDexCode();
- timing.begin("Convert to event based debug info");
EventBasedDebugInfo debugInfo =
getEventBasedDebugInfo(method.getDefinition(), dexCode, appView);
- timing.end();
IntBox firstDefaultEventPc = new IntBox(-1);
Pair<Integer, Position> lastPosition = new Pair<>();
DexDebugEventVisitor visitor =
@@ -69,21 +62,17 @@
getCurrentPc(),
lastPosition.getSecond(),
positionRemapper,
- mappedPositions,
- timing);
+ mappedPositions);
}
lastPosition.setFirst(getCurrentPc());
lastPosition.setSecond(currentPosition);
}
};
- timing.begin("Visit events");
for (DexDebugEvent event : debugInfo.events) {
event.accept(visitor);
}
- timing.end();
- timing.begin("Flush");
int lastInstructionPc = DebugRepresentation.getLastExecutableInstruction(dexCode).getOffset();
if (lastPosition.getSecond() != null) {
remapAndAddForPc(
@@ -92,18 +81,13 @@
lastInstructionPc + 1,
lastPosition.getSecond(),
positionRemapper,
- mappedPositions,
- timing);
+ mappedPositions);
}
- timing.end();
assert !mappedPositions.isEmpty()
|| dexCode.instructions.length == 1
|| !dexCode.hasThrowingInstructions();
- timing.begin("Record pc mapping");
pcBasedDebugInfo.recordPcMappingFor(method, pcEncodingCutoff);
- timing.end();
- timing.end();
return mappedPositions;
}
@@ -112,19 +96,14 @@
int startPc,
int endPc,
Position position,
- MethodPositionRemapper remapper,
- List<MappedPosition> mappedPositions,
- Timing timing) {
- timing.begin("Remap position");
+ ClassPositionRemapper remapper,
+ List<MappedPosition> mappedPositions) {
Pair<Position, Position> remappedPosition = remapper.createRemappedPosition(position);
- timing.end();
- timing.begin("Update mapped positions");
Position oldPosition = remappedPosition.getFirst();
for (int currentPc = startPc; currentPc < endPc; currentPc++) {
mappedPositions.add(
new MappedPosition(oldPosition, debugInfoProvider.getPcEncoding(currentPc)));
}
- timing.end();
}
// This conversion *always* creates an event based debug info encoding as any non-info will
diff --git a/src/main/java/com/android/tools/r8/utils/positions/LineNumberOptimizer.java b/src/main/java/com/android/tools/r8/utils/positions/LineNumberOptimizer.java
index 02a46e9..aa55443 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/LineNumberOptimizer.java
@@ -36,7 +36,6 @@
import com.android.tools.r8.utils.timing.Timing;
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
@@ -121,8 +120,6 @@
// Collect which files contain which classes that need to have their line numbers optimized.
timing.begin("Process classes");
- AppPositionRemapper positionRemapper =
- AppPositionRemapper.create(appView, cfLineToMethodMapper);
for (DexProgramClass clazz : appView.appInfo().classes()) {
if (shouldRun(clazz, appView)) {
runForClass(
@@ -130,7 +127,7 @@
appView,
representation,
builder,
- positionRemapper,
+ cfLineToMethodMapper,
positionToMappedRangeMapper,
timing);
}
@@ -159,7 +156,7 @@
AppView<?> appView,
DebugRepresentationPredicate representation,
MappedPositionToClassNameMapperBuilder builder,
- AppPositionRemapper positionRemapper,
+ CfLineToMethodMapper cfLineToMethodMapper,
PositionToMappedRangeMapper positionToMappedRangeMapper,
Timing timing) {
timing.begin("Prelude");
@@ -173,8 +170,6 @@
renamedMethodNames.sort(DexString::compareTo);
timing.end();
- ClassPositionRemapper classPositionRemapper =
- positionRemapper.createClassPositionRemapper(clazz);
for (DexString methodName : renamedMethodNames) {
List<ProgramMethod> methods = methodsByRenamedName.get(methodName);
if (methods.size() > 1) {
@@ -190,66 +185,62 @@
assert verifyMethodsAreKeptDirectlyOrIndirectly(appView, methods);
}
+ ClassPositionRemapper positionRemapper =
+ ClassPositionRemapper.getPositionRemapper(appView, cfLineToMethodMapper);
+
timing.begin("Process methods");
- // We must reuse the same MethodPositionRemapper for methods with the same name.
- MethodPositionRemapper methodPositionRemapper =
- classPositionRemapper.createMethodPositionRemapper(methods);
for (ProgramMethod method : methods) {
- if (shouldRunForMethod(method, appView, methodName, methods)) {
- List<MappedPosition> mappedPositions =
- runForMethod(
- method,
- appView,
- methods,
- methodPositionRemapper,
- positionToMappedRangeMapper,
- representation,
- timing);
- timing.begin("Add mapped positions");
- boolean canUseDexPc =
- methods.size() == 1 && representation.getDexPcEncodingCutoff(method) > 0;
- classNamingBuilder.addMappedPositions(
- method, mappedPositions, methodPositionRemapper, canUseDexPc);
- timing.end();
- }
+ runForMethod(
+ method,
+ appView,
+ classNamingBuilder,
+ methodName,
+ methods,
+ positionRemapper,
+ positionToMappedRangeMapper,
+ representation,
+ timing);
}
timing.end();
} // for each method group, grouped by name
}
- private static boolean shouldRunForMethod(
- ProgramMethod method, AppView<?> appView, DexString methodName, List<ProgramMethod> methods) {
- DexEncodedMethod definition = method.getDefinition();
- return !method.getName().isIdenticalTo(methodName)
- || mustHaveResidualDebugInfo(appView.options(), definition)
- || definition.isD8R8Synthesized()
- || methods.size() > 1;
- }
-
- private static List<MappedPosition> runForMethod(
+ private static void runForMethod(
ProgramMethod method,
AppView<?> appView,
+ MappedPositionToClassNamingBuilder classNamingBuilder,
+ DexString methodName,
List<ProgramMethod> methods,
- MethodPositionRemapper positionRemapper,
+ ClassPositionRemapper positionRemapper,
PositionToMappedRangeMapper positionToMappedRangeMapper,
DebugRepresentationPredicate representation,
Timing timing) {
+ DexEncodedMethod definition = method.getDefinition();
+ if (method.getName().isIdenticalTo(methodName)
+ && !mustHaveResidualDebugInfo(appView.options(), definition)
+ && !definition.isD8R8Synthesized()
+ && methods.size() <= 1) {
+ return;
+ }
+ positionRemapper.setCurrentMethod(definition);
List<MappedPosition> mappedPositions;
int pcEncodingCutoff = methods.size() == 1 ? representation.getDexPcEncodingCutoff(method) : -1;
boolean canUseDexPc = pcEncodingCutoff > 0;
- Code code = method.getDefinition().getCode();
- if (code != null
- && (code.isCfCode() || code.isDexCode())
+ if (definition.getCode() != null
+ && (definition.getCode().isCfCode() || definition.getCode().isDexCode())
&& !appView.isCfByteCodePassThrough(method)) {
timing.begin("Get mapped positions");
mappedPositions =
positionToMappedRangeMapper.getMappedPositions(
- method, positionRemapper, methods.size() > 1, canUseDexPc, pcEncodingCutoff, timing);
+ method, positionRemapper, methods.size() > 1, canUseDexPc, pcEncodingCutoff);
timing.end();
} else {
- mappedPositions = Collections.emptyList();
+ mappedPositions = new ArrayList<>();
}
- return mappedPositions;
+
+ timing.begin("Add mapped positions");
+ classNamingBuilder.addMappedPositions(method, mappedPositions, positionRemapper, canUseDexPc);
+ timing.end();
}
@SuppressWarnings("ComplexBooleanConstant")
diff --git a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
index 3530795..b854592 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
@@ -220,7 +220,7 @@
public MappedPositionToClassNamingBuilder addMappedPositions(
ProgramMethod method,
List<MappedPosition> mappedPositions,
- MethodPositionRemapper positionRemapper,
+ ClassPositionRemapper positionRemapper,
boolean canUseDexPc) {
DexEncodedMethod definition = method.getDefinition();
DexMethod residualMethod =
diff --git a/src/main/java/com/android/tools/r8/utils/positions/MethodPositionRemapper.java b/src/main/java/com/android/tools/r8/utils/positions/MethodPositionRemapper.java
deleted file mode 100644
index 98e1b50..0000000
--- a/src/main/java/com/android/tools/r8/utils/positions/MethodPositionRemapper.java
+++ /dev/null
@@ -1,12 +0,0 @@
-// Copyright (c) 2025, the R8 project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-package com.android.tools.r8.utils.positions;
-
-import com.android.tools.r8.ir.code.Position;
-import com.android.tools.r8.utils.Pair;
-
-public interface MethodPositionRemapper {
-
- Pair<Position, Position> createRemappedPosition(Position position);
-}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java b/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java
index ba0e2ea..10a79ac 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java
@@ -9,7 +9,6 @@
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.graph.DexDebugInfo;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.utils.timing.Timing;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -20,11 +19,10 @@
List<MappedPosition> getMappedPositions(
ProgramMethod method,
- MethodPositionRemapper positionRemapper,
+ ClassPositionRemapper positionRemapper,
boolean hasOverloads,
boolean canUseDexPc,
- int pcEncodingCutoff,
- Timing timing);
+ int pcEncodingCutoff);
void updateDebugInfoInCodeObjects();
@@ -53,15 +51,13 @@
@Override
public List<MappedPosition> getMappedPositions(
ProgramMethod method,
- MethodPositionRemapper positionRemapper,
+ ClassPositionRemapper positionRemapper,
boolean hasOverloads,
boolean canUseDexPc,
- int pcEncodingCutoff,
- Timing timing) {
+ int pcEncodingCutoff) {
return canUseDexPc
- ? pcMapper.optimizeDexCodePositionsForPc(
- method, positionRemapper, pcEncodingCutoff, timing)
- : noPcMapper.optimizeDexCodePositions(method, positionRemapper, timing);
+ ? pcMapper.optimizeDexCodePositionsForPc(method, positionRemapper, pcEncodingCutoff)
+ : noPcMapper.optimizeDexCodePositions(method, positionRemapper);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/positions/PositionUtils.java b/src/main/java/com/android/tools/r8/utils/positions/PositionUtils.java
index 3ee5a32..9852eeb 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/PositionUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/PositionUtils.java
@@ -20,7 +20,7 @@
public class PositionUtils {
public static Position remapAndAdd(
- Position position, MethodPositionRemapper remapper, List<MappedPosition> mappedPositions) {
+ Position position, ClassPositionRemapper remapper, List<MappedPosition> mappedPositions) {
Pair<Position, Position> remappedPosition = remapper.createRemappedPosition(position);
Position oldPosition = remappedPosition.getFirst();
Position newPosition = remappedPosition.getSecond();
diff --git a/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java b/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java
index 5a1910e..dc7a609 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java
@@ -329,7 +329,7 @@
ThrowableConsumer<? super R8FullTestBuilder> configuration) {
return environment ->
BenchmarkBase.runner(environment)
- .setWarmupIterations(0)
+ .setWarmupIterations(builder.warmupIterations)
.run(
results -> {
CompilerDump dump = builder.getExtractedDump(environment);
