Skip to content
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

WeakHashMap enhancement and expand #2632

Merged
merged 4 commits into from Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -33,8 +33,9 @@ class AndroidObjectInspectorsTest {
it.originObject.type == INSTANCE
&& it.owningClassSimpleName == "Recomposer"
}
assertThat(recomposerNode.originObject.leakingStatus == NOT_LEAKING)
assertThat(recomposerNode.originObject.leakingStatusReason == "Recomposer is in state PendingWork")
assertThat(recomposerNode.originObject.leakingStatus).isEqualTo(NOT_LEAKING)
assertThat(recomposerNode.originObject.leakingStatusReason)
.isEqualTo("Recomposer is in state PendingWork")
}

@Test fun `COMPOSITION_IMPL leaking status relies on disposal`() {
Expand All @@ -61,7 +62,7 @@ class AndroidObjectInspectorsTest {
.referencePath.single {
it.owningClassSimpleName == "CompositionImpl"
}
assertThat(recomposerNode.originObject.leakingStatus == LEAKING)
assertThat(recomposerNode.originObject.leakingStatusReason == "Composition disposed")
assertThat(recomposerNode.originObject.leakingStatus).isEqualTo(LEAKING)
assertThat(recomposerNode.originObject.leakingStatusReason).isEqualTo("Composition disposed")
}
}
6 changes: 3 additions & 3 deletions shark/shark-android/src/test/java/shark/HprofIOPerfTest.kt
Expand Up @@ -179,7 +179,7 @@ class HprofIOPerfTest {
)
.isEqualTo(
listOf(
24384, 40.0, 1244990, 25653, 40.0, 1302298
24387, 40.0, 1245178, 25656, 40.0, 1302486
)
)
}
Expand All @@ -197,7 +197,7 @@ class HprofIOPerfTest {
)
.isEqualTo(
listOf(
22472, 40.0, 2202271, 22477, 40.0, 2202451
22477, 40.0, 2202671, 22482, 40.0, 2202851
)
)
}
Expand All @@ -215,7 +215,7 @@ class HprofIOPerfTest {
)
.isEqualTo(
listOf(
16829, 32.0, 765450, 16831, 32.0, 765514
16835, 32.0, 765930, 16837, 32.0, 765994
)
)
}
Expand Down
2 changes: 2 additions & 0 deletions shark/shark/api/shark.api
Expand Up @@ -39,6 +39,7 @@ public abstract class shark/ApacheHarmonyInstanceRefReaders : java/lang/Enum, sh
public static final field HASH_MAP Lshark/ApacheHarmonyInstanceRefReaders;
public static final field HASH_SET Lshark/ApacheHarmonyInstanceRefReaders;
public static final field LINKED_LIST Lshark/ApacheHarmonyInstanceRefReaders;
public static final field WEAK_HASH_MAP Lshark/ApacheHarmonyInstanceRefReaders;
public synthetic fun <init> (Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public static fun valueOf (Ljava/lang/String;)Lshark/ApacheHarmonyInstanceRefReaders;
public static fun values ()[Lshark/ApacheHarmonyInstanceRefReaders;
Expand Down Expand Up @@ -572,6 +573,7 @@ public abstract class shark/OpenJdkInstanceRefReaders : java/lang/Enum, shark/Ch
public static final field HASH_MAP Lshark/OpenJdkInstanceRefReaders;
public static final field HASH_SET Lshark/OpenJdkInstanceRefReaders;
public static final field LINKED_LIST Lshark/OpenJdkInstanceRefReaders;
public static final field WEAK_HASH_MAP Lshark/OpenJdkInstanceRefReaders;
public synthetic fun <init> (Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public static fun valueOf (Ljava/lang/String;)Lshark/OpenJdkInstanceRefReaders;
public static fun values ()[Lshark/OpenJdkInstanceRefReaders;
Expand Down
@@ -1,11 +1,12 @@
package shark

import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory
import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader
import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory
import shark.HeapObject.HeapInstance
import shark.internal.InternalSharedArrayListReferenceReader
import shark.internal.InternalSharedHashMapReferenceReader
import shark.internal.InternalSharedLinkedListReferenceReader
import shark.internal.InternalSharedWeakHashMapReferenceReader

/**
* Defines [VirtualInstanceReferenceReader] factories for common Apache Harmony data structures.
Expand Down Expand Up @@ -116,6 +117,29 @@ enum class ApacheHarmonyInstanceRefReaders : OptionalFactory {
}
},

// https://cs.android.com/android/platform/superproject/+/android-6.0.1_r81:libcore/luni/src/main/java/java/util/WeakHashMap.java
WEAK_HASH_MAP {
override fun create(graph: HeapGraph): VirtualInstanceReferenceReader? {
val weakHashMapClass = graph.findClassByName("java.util.WeakHashMap") ?: return null

// No table field in Apache Harmony impl.
val isOpenJdkImpl = weakHashMapClass.readRecordFields()
.any { weakHashMapClass.instanceFieldName(it) == "table" }

if (isOpenJdkImpl) {
return null
}

return InternalSharedWeakHashMapReferenceReader(
classObjectId = weakHashMapClass.objectId,
tableFieldName = "elementData",
isEntryWithNullKey = { entry ->
entry["java.util.WeakHashMap\$Entry", "isNull"]!!.value.asBoolean == true
},
)
}
},

// https://cs.android.com/android/platform/superproject/+/android-6.0.1_r81:libcore/luni/src/main/java/java/util/HashSet.java
/**
* Handles HashSet & LinkedHashSet
Expand Down
29 changes: 28 additions & 1 deletion shark/shark/src/main/java/shark/OpenJdkInstanceRefReaders.kt
@@ -1,11 +1,12 @@
package shark

import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory
import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader
import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory
import shark.HeapObject.HeapInstance
import shark.internal.InternalSharedArrayListReferenceReader
import shark.internal.InternalSharedHashMapReferenceReader
import shark.internal.InternalSharedLinkedListReferenceReader
import shark.internal.InternalSharedWeakHashMapReferenceReader

/**
* Defines [VirtualInstanceReferenceReader] factories for common OpenJDK data structures.
Expand Down Expand Up @@ -158,6 +159,32 @@ enum class OpenJdkInstanceRefReaders : OptionalFactory {
}
},

// https://cs.android.com/android/platform/superproject/main/+/main:libcore/ojluni/src/main/java/java/util/WeakHashMap.java
WEAK_HASH_MAP {
override fun create(graph: HeapGraph): VirtualInstanceReferenceReader? {
val weakHashMapClass = graph.findClassByName("java.util.WeakHashMap") ?: return null

// No table field in Apache Harmony impl.
val isOpenJdkImpl = weakHashMapClass.readRecordFields()
.any { weakHashMapClass.instanceFieldName(it) == "table" }

if (!isOpenJdkImpl) {
return null
}

val nullKeyObjectId = weakHashMapClass.readStaticField("NULL_KEY")!!.value.asObjectId

return InternalSharedWeakHashMapReferenceReader(
classObjectId = weakHashMapClass.objectId,
tableFieldName = "table",
isEntryWithNullKey = { entry ->
val key = entry["java.lang.ref.Reference", "referent"]!!.value
key.asObjectId == nullKeyObjectId
},
)
}
},

/**
* Handles HashSet & LinkedHashSet
*/
Expand Down
Expand Up @@ -103,6 +103,70 @@ internal class InternalSharedHashMapReferenceReader(
}
}

internal class InternalSharedWeakHashMapReferenceReader(
private val classObjectId: Long,
private val tableFieldName: String,
private val isEntryWithNullKey: (HeapInstance) -> Boolean,
) : VirtualInstanceReferenceReader {
override fun matches(instance: HeapInstance): Boolean {
return instance.instanceClassId == classObjectId
}

override fun read(source: HeapInstance): Sequence<Reference> {
val table = source["java.util.WeakHashMap", tableFieldName]!!.valueAsObjectArray
return if (table != null) {
val entries = table.readElements().mapNotNull { entryRef ->
if (entryRef.isNonNullReference) {
val entry = entryRef.asObject!!.asInstance!!
generateSequence(entry) { node ->
node["java.util.WeakHashMap\$Entry", "next"]!!.valueAsInstance
}
} else {
null
}
}.flatten()

val declaringClassId = source.instanceClassId

entries.flatMap { entry ->
val key = if (isEntryWithNullKey(entry)) {
null
} else {
entry["java.lang.ref.Reference", "referent"]!!.value
}
if (key?.isNullReference == true) {
return@flatMap emptySequence() // cleared key
}
val value = entry["java.util.WeakHashMap\$Entry", "value"]!!.value
val valueRef = if (value.isNonNullReference) {
Reference(
valueObjectId = value.asObjectId!!,
isLowPriority = false,
lazyDetailsResolver = {
val keyAsString = key?.asObject?.asInstance?.readAsJavaString()?.let { "\"$it\"" }
val keyAsName = keyAsString ?: key?.asObject?.toString() ?: "null"
LazyDetails(
name = keyAsName,
locationClassObjectId = declaringClassId,
locationType = ARRAY_ENTRY,
isVirtual = true,
matchedLibraryLeak = null
)
}
)
} else null
if (valueRef != null) {
sequenceOf(valueRef)
} else {
emptySequence()
}
}
} else {
emptySequence()
}
}
}

internal class InternalSharedArrayListReferenceReader(
private val className: String,
private val classObjectId: Long,
Expand Down