Skip to content

Commit d53aacd

Browse files
committed
perf: Free memory earlier and remove negligible lookup maps
Negligible lookup maps used for matching fingerprints have been removed to reduce the likelihood of OOM when the maps are instantiated, commonly observed with 400M RAM. Additionally, lookup maps previously kept for the duration of the patcher instance are now cleared before the classes are compiled. This reduces the likelihood of OOM when compiling classes. On a related note, a linear increase in memory usage is observed with every compiled class until all classes are compiled implying compiled classes not being freed by GC because they are still referenced. After compiling a class, the class is technically free-able though. The classes are assumed to be referenced in the `multidexlib2` library that takes the list of all classes to compile multiple DEX with and seems to hold the reference to all these classes in memory until all DEX are compiled. A clever fix would involve splitting the list of classes into chunks and getting rid of the list of all classes so that after every DEX compilation, the corresponding split of classes can be freed.
1 parent f1615b7 commit d53aacd

File tree

5 files changed

+44
-94
lines changed

5 files changed

+44
-94
lines changed

src/main/kotlin/app/revanced/patcher/Fingerprint.kt

+5-26
Original file line numberDiff line numberDiff line change
@@ -66,36 +66,15 @@ class Fingerprint internal constructor(
6666
}
6767

6868
// TODO: If only one string is necessary, why not use a single string for every fingerprint?
69-
fun Fingerprint.lookupByStrings() = strings?.firstNotNullOfOrNull { lookupMaps.methodsByStrings[it] }
70-
if (lookupByStrings()?.let(::match) == true) {
69+
if (strings?.firstNotNullOfOrNull { lookupMaps.methodsByStrings[it] }?.let(::match) == true) {
7170
return true
7271
}
7372

74-
// No strings declared or none matched (partial matches are allowed).
75-
// Use signature matching.
76-
fun Fingerprint.lookupBySignature(): MethodClassPairs {
77-
if (accessFlags == null) return lookupMaps.allMethods
78-
79-
var returnTypeValue = returnType
80-
if (returnTypeValue == null) {
81-
if (AccessFlags.CONSTRUCTOR.isSet(accessFlags)) {
82-
// Constructors always have void return type.
83-
returnTypeValue = "V"
84-
} else {
85-
return lookupMaps.allMethods
86-
}
87-
}
88-
89-
val signature =
90-
buildString {
91-
append(accessFlags)
92-
append(returnTypeValue.first())
93-
appendParameters(parameters ?: return@buildString)
94-
}
95-
96-
return lookupMaps.methodsBySignature[signature] ?: return MethodClassPairs()
73+
context.classes.forEach { classDef ->
74+
if (match(context, classDef)) return true
9775
}
98-
return match(lookupBySignature())
76+
77+
return false
9978
}
10079

10180
/**

src/main/kotlin/app/revanced/patcher/Patcher.kt

+5-7
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,12 @@ class Patcher(private val config: PatcherConfig) : Closeable {
9898

9999
logger.info("Merging extensions")
100100

101-
context.executablePatches.forEachRecursively { patch ->
102-
if (patch is BytecodePatch && patch.extension != null) {
103-
context.bytecodeContext.merge(patch.extension)
104-
}
105-
}
101+
with(context.bytecodeContext) {
102+
context.executablePatches.mergeExtensions()
106103

107-
// Initialize lookup maps.
108-
context.bytecodeContext.lookupMaps
104+
// Initialize lookup maps.
105+
lookupMaps
106+
}
109107

110108
logger.info("Executing patches")
111109

src/main/kotlin/app/revanced/patcher/PatcherConfig.kt

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class PatcherConfig(
2020
private val temporaryFilesPath: File = File("revanced-temporary-files"),
2121
aaptBinaryPath: String? = null,
2222
frameworkFileDirectory: String? = null,
23+
@Deprecated("This is going to be removed in the future because it is not needed anymore.")
2324
internal val multithreadingDexFileWriter: Boolean = false,
2425
) {
2526
private val logger = Logger.getLogger(PatcherConfig::class.java.name)

src/main/kotlin/app/revanced/patcher/patch/BytecodePatchContext.kt

+30-61
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import lanchon.multidexlib2.MultiDexIO
2121
import lanchon.multidexlib2.RawDexIO
2222
import java.io.Closeable
2323
import java.io.FileFilter
24-
import java.io.InputStream
2524
import java.util.*
2625
import java.util.logging.Logger
2726

@@ -60,40 +59,41 @@ class BytecodePatchContext internal constructor(private val config: PatcherConfi
6059
internal val lookupMaps by lazy { LookupMaps(classes) }
6160

6261
/**
63-
* A map for lookup by [merge].
62+
* Merge the extensions for this set of patches.
6463
*/
65-
internal val classesByType = mutableMapOf<String, ClassDef>().apply {
66-
classes.forEach { classDef -> put(classDef.type, classDef) }
67-
}
64+
internal fun Set<Patch<*>>.mergeExtensions() {
65+
// Lookup map for fast checking if a class exists by its type.
66+
val classesByType = mutableMapOf<String, ClassDef>().apply {
67+
classes.forEach { classDef -> put(classDef.type, classDef) }
68+
}
6869

69-
/**
70-
* Merge an extension to [classes].
71-
*
72-
* @param extensionInputStream The input stream of the extension to merge.
73-
*/
74-
internal fun merge(extensionInputStream: InputStream) {
75-
val extension = extensionInputStream.readAllBytes()
70+
forEachRecursively { patch ->
71+
if (patch is BytecodePatch && patch.extension != null) {
7672

77-
RawDexIO.readRawDexFile(extension, 0, null).classes.forEach { classDef ->
78-
val existingClass = classesByType[classDef.type] ?: run {
79-
logger.fine("Adding class \"$classDef\"")
73+
val extension = patch.extension.readAllBytes()
8074

81-
classes += classDef
82-
classesByType[classDef.type] = classDef
75+
RawDexIO.readRawDexFile(extension, 0, null).classes.forEach { classDef ->
76+
val existingClass = classesByType[classDef.type] ?: run {
77+
logger.fine("Adding class \"$classDef\"")
8378

84-
return@forEach
85-
}
79+
classes += classDef
80+
classesByType[classDef.type] = classDef
81+
82+
return@forEach
83+
}
8684

87-
logger.fine("Class \"$classDef\" exists already. Adding missing methods and fields.")
85+
logger.fine("Class \"$classDef\" exists already. Adding missing methods and fields.")
8886

89-
existingClass.merge(classDef, this@BytecodePatchContext).let { mergedClass ->
90-
// If the class was merged, replace the original class with the merged class.
91-
if (mergedClass === existingClass) {
92-
return@let
93-
}
87+
existingClass.merge(classDef, this@BytecodePatchContext).let { mergedClass ->
88+
// If the class was merged, replace the original class with the merged class.
89+
if (mergedClass === existingClass) {
90+
return@let
91+
}
9492

95-
classes -= existingClass
96-
classes += mergedClass
93+
classes -= existingClass
94+
classes += mergedClass
95+
}
96+
}
9797
}
9898
}
9999
}
@@ -145,6 +145,9 @@ class BytecodePatchContext internal constructor(private val config: PatcherConfi
145145
override fun get(): Set<PatcherResult.PatchedDexFile> {
146146
logger.info("Compiling patched dex files")
147147

148+
// Free up memory before compiling the dex files.
149+
lookupMaps.close()
150+
148151
val patchedDexFileResults =
149152
config.patchedFiles.resolve("dex").also {
150153
it.deleteRecursively() // Make sure the directory is empty.
@@ -178,21 +181,6 @@ class BytecodePatchContext internal constructor(private val config: PatcherConfi
178181
* @param classes The list of classes to create the lookup maps from.
179182
*/
180183
internal class LookupMaps internal constructor(classes: List<ClassDef>) : Closeable {
181-
/**
182-
* Classes associated by their type.
183-
*/
184-
internal val classesByType = classes.associateBy { it.type }.toMutableMap()
185-
186-
/**
187-
* All methods and the class they are a member of.
188-
*/
189-
internal val allMethods = MethodClassPairs()
190-
191-
/**
192-
* Methods associated by its access flags, return type and parameter.
193-
*/
194-
internal val methodsBySignature = MethodClassPairsLookupMap()
195-
196184
/**
197185
* Methods associated by strings referenced in it.
198186
*/
@@ -203,22 +191,6 @@ class BytecodePatchContext internal constructor(private val config: PatcherConfi
203191
classDef.methods.forEach { method ->
204192
val methodClassPair: MethodClassPair = method to classDef
205193

206-
// For fingerprints with no access or return type specified.
207-
allMethods += methodClassPair
208-
209-
val accessFlagsReturnKey = method.accessFlags.toString() + method.returnType.first()
210-
211-
// Add <access><returnType> as the key.
212-
methodsBySignature[accessFlagsReturnKey] = methodClassPair
213-
214-
// Add <access><returnType>[parameters] as the key.
215-
methodsBySignature[
216-
buildString {
217-
append(accessFlagsReturnKey)
218-
appendParameters(method.parameterTypes)
219-
},
220-
] = methodClassPair
221-
222194
// Add strings contained in the method as the key.
223195
method.instructionsOrNull?.forEach instructions@{ instruction ->
224196
if (instruction.opcode != Opcode.CONST_STRING && instruction.opcode != Opcode.CONST_STRING_JUMBO) {
@@ -259,15 +231,12 @@ class BytecodePatchContext internal constructor(private val config: PatcherConfi
259231
}
260232

261233
override fun close() {
262-
allMethods.clear()
263-
methodsBySignature.clear()
264234
methodsByStrings.clear()
265235
}
266236
}
267237

268238
override fun close() {
269239
lookupMaps.close()
270-
classesByType.clear()
271240
classes.clear()
272241
}
273242
}

src/test/kotlin/app/revanced/patcher/PatcherTest.kt

+3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import app.revanced.patcher.util.ProxyClassList
66
import com.android.tools.smali.dexlib2.immutable.ImmutableClassDef
77
import com.android.tools.smali.dexlib2.immutable.ImmutableMethod
88
import io.mockk.every
9+
import io.mockk.just
910
import io.mockk.mockk
11+
import io.mockk.runs
1012
import kotlinx.coroutines.flow.toList
1113
import kotlinx.coroutines.runBlocking
1214
import org.junit.jupiter.api.BeforeEach
@@ -193,6 +195,7 @@ internal object PatcherTest {
193195
private operator fun Set<Patch<*>>.invoke(): List<PatchResult> {
194196
every { patcher.context.executablePatches } returns toMutableSet()
195197
every { patcher.context.bytecodeContext.lookupMaps } returns LookupMaps(patcher.context.bytecodeContext.classes)
198+
every { with(patcher.context.bytecodeContext) { any<Set<Patch<*>>>().mergeExtensions() } } just runs
196199

197200
return runBlocking { patcher().toList() }
198201
}

0 commit comments

Comments
 (0)