Skip to content

Commit b419332

Browse files
eymarMatkovIvan
eymar
and
MatkovIvan
authored
Implement WeakKeysCache for web (JetBrains#2632)
Implement WeakKeysCache for web Fixes [CMP-9410](https://youtrack.jetbrains.com/issue/CMP-9410) Other changes: - Also change the constraint of Values types from Any? to Any, as WeakKeysCache is used only with non-nullable values - Get rid of `interface Cache` and make `fun getOrPut` inline to avoid lambda instantiation on every getOrPut call ## Testing Added the related tests. But without explcit GC calls - it's not exposed. ## Release Notes N/A --------- Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>
1 parent b168065 commit b419332

11 files changed

Lines changed: 191 additions & 43 deletions

File tree

‎compose/ui/ui-text/build.gradle‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,10 @@ if(AndroidXComposePlugin.isMultiplatformEnabled(project)) {
235235
dependsOn(skikoTest)
236236
}
237237
nativeTest.dependsOn(nonJvmTest)
238-
jsTest.dependsOn(nonJvmTest)
239-
wasmJsTest.dependsOn(nonJvmTest)
238+
239+
webTest.dependsOn(nonJvmTest)
240+
jsTest.dependsOn(webTest)
241+
wasmJsTest.dependsOn(webTest)
240242

241243
configureEach {
242244
languageSettings.optIn("androidx.compose.ui.text.ExperimentalTextApi")

‎compose/ui/ui-text/src/desktopMain/kotlin/androidx/compose/ui/text/Cache.jvm.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
package androidx.compose.ui.text
1818

1919

20-
internal actual class WeakKeysCache<K : Any, V> : Cache<K, V> {
20+
internal actual class WeakKeysCache<K : Any, V: Any> {
2121
private val cache = java.util.WeakHashMap<K, V>()
2222

23-
actual override fun get(key: K, loader: (K) -> V): V =
23+
actual inline fun getOrPut(key: K, loader: (K) -> V): V =
2424
cache.getOrPut(key) { loader(key) }
2525
}

‎compose/ui/ui-text/src/desktopTest/kotlin/androidx/compose/ui/text/WeakKeysCacheTest.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ class WeakKeysCacheTest {
2626
fun clearOnGC() {
2727
val cache = WeakKeysCache<MyKey, Int>()
2828
var created = 0
29-
assertEquals(1, cache.get(MyKey(42)) { ++created })
30-
assertEquals(1, cache.get(MyKey(42)) { ++created })
29+
assertEquals(1, cache.getOrPut(MyKey(42)) { ++created })
30+
assertEquals(1, cache.getOrPut(MyKey(42)) { ++created })
3131
System.gc()
32-
assertEquals(2, cache.get(MyKey(42)) { ++created })
33-
assertEquals(2, cache.get(MyKey(42)) { ++created })
32+
assertEquals(2, cache.getOrPut(MyKey(42)) { ++created })
33+
assertEquals(2, cache.getOrPut(MyKey(42)) { ++created })
3434
}
3535
}

‎compose/ui/ui-text/src/nativeMain/kotlin/androidx/compose/ui/text/Cache.native.kt‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,33 @@ package androidx.compose.ui.text
1919
import kotlin.experimental.ExperimentalNativeApi
2020
import kotlin.native.ref.WeakReference
2121

22-
internal actual class WeakKeysCache<K : Any, V> : Cache<K, V> {
22+
internal actual class WeakKeysCache<K : Any, V: Any> {
2323
// TODO Use WeakHashMap once available https://youtrack.jetbrains.com/issue/KT-48075
2424
private val cache = HashMap<Key<K>, V>()
2525

26-
actual override fun get(key: K, loader: (K) -> V): V {
26+
actual inline fun getOrPut(key: K, loader: (K) -> V): V {
2727
cache.entries.removeAll { !it.key.isAvailable }
2828
return cache.getOrPut(Key(key)) { loader(key) }
2929
}
3030

3131
@OptIn(ExperimentalNativeApi::class)
32-
private class Key<K : Any>(key: K) {
32+
internal class Key<K : Any>(key: K) {
3333
private val ref = WeakReference(key)
3434
private val hash: Int = key.hashCode()
3535

3636
val isAvailable get() = ref.value != null
3737

3838
override fun equals(other: Any?): Boolean {
3939
if (this === other) return true
40+
if (other == null) return false
4041
other as Key<*>
41-
return ref.value == other.ref.value
42+
val a = ref.get()
43+
val b = other.ref.get()
44+
if (a == null || b == null) {
45+
// If either side is cleared, they should not be considered equal
46+
return false
47+
}
48+
return a == b
4249
}
4350

4451
override fun hashCode(): Int = hash

‎compose/ui/ui-text/src/nativeTest/kotlin/androidx/compose/ui/text/WeakKeysCacheTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ class WeakKeysCacheTest {
3333
// Accessing to weak reference creates strong one in stack now.
3434
// So to make GC work, wrap it into separate function.
3535
fun checkCache(expected: Int) {
36-
assertEquals(expected, cache.get(MyKey(42)) { ++created })
37-
assertEquals(expected, cache.get(MyKey(42)) { ++created })
36+
assertEquals(expected, cache.getOrPut(MyKey(42)) { ++created })
37+
assertEquals(expected, cache.getOrPut(MyKey(42)) { ++created })
3838
}
3939
checkCache(1)
4040
GC.collect()

‎compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/Cache.skiko.kt‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,11 @@ package androidx.compose.ui.text
1818

1919
import org.jetbrains.skiko.currentNanoTime
2020

21-
// Extremely simple Cache interface which is enough for ui.text needs
22-
internal interface Cache<K, V> {
23-
// get a value for [key] or load it by [loader] if it doesn't exist
24-
fun get(key: K, loader: (K) -> V): V
25-
}
26-
2721
/**
2822
* Cache with weak keys.
2923
*/
30-
internal expect class WeakKeysCache<K : Any, V>() : Cache<K, V> {
31-
override fun get(key: K, loader: (K) -> V): V
24+
internal expect class WeakKeysCache<K : Any, V: Any>() {
25+
inline fun getOrPut(key: K, loader: (K) -> V): V
3226
}
3327

3428
/**
@@ -37,11 +31,11 @@ internal expect class WeakKeysCache<K : Any, V>() : Cache<K, V> {
3731
internal class ExpireAfterAccessCache<K, V>(
3832
val expireAfterNanos: Long,
3933
val currentNanos: () -> Long = ::currentNanoTime
40-
) : Cache<K, V> {
34+
) {
4135
internal val map = HashMap<K, V>()
4236
internal val accessTime = LinkedHashMap<K, Long>()
4337

44-
override fun get(key: K, loader: (K) -> V): V {
38+
inline fun getOrPut(key: K, loader: (K) -> V): V {
4539
accessTime.remove(key)
4640
return map.getOrPut(key) {
4741
loader(key)

‎compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/platform/ParagraphBuilder.skiko.kt‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,13 @@ internal class ParagraphBuilder(
435435
op.style.fontStyle ?: FontStyle.Normal,
436436
op.style.fontSynthesis ?: FontSynthesis.All
437437
)
438-
pb.pushStyle(makeSkTextStyle(op.style.toImmutable()))
438+
439+
// It's always mutable at this point, so we can safely cast
440+
val style = (op.style as ComputedStyle.Mutable).toImmutable()
441+
// Store immutable reference because it's used as a weak reference key
442+
op.style = style
443+
444+
pb.pushStyle(makeSkTextStyle(style))
439445
}
440446
is Op.PutPlaceholder -> {
441447
val placeholderStyle =
@@ -471,7 +477,7 @@ internal class ParagraphBuilder(
471477

472478
data class StyleAdd(
473479
override val position: Int,
474-
val style: ComputedStyle.Mutable
480+
var style: ComputedStyle
475481
) : Op()
476482

477483
data class PutPlaceholder(
@@ -544,7 +550,9 @@ internal class ParagraphBuilder(
544550
)
545551
)
546552
} else {
547-
prev.style.merge(density, cut.style)
553+
// It's always mutable at this point, so we can safely cast
554+
val style = prev.style as ComputedStyle.Mutable
555+
style.merge(density, cut.style)
548556
}
549557
}
550558
is Cut.StyleRemove -> {
@@ -642,7 +650,7 @@ internal class ParagraphBuilder(
642650
}
643651

644652
private fun makeSkTextStyle(style: ComputedStyle.Immutable): SkTextStyle {
645-
return skTextStylesCache.get(style) {
653+
return skTextStylesCache.getOrPut(style) {
646654
it.toSkTextStyle(fontFamilyResolver)
647655
}
648656
}

‎compose/ui/ui-text/src/skikoMain/kotlin/androidx/compose/ui/text/platform/PlatformFont.skiko.kt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package androidx.compose.ui.text.platform
1717

1818
import org.jetbrains.skia.Typeface as SkTypeface
19-
import androidx.compose.ui.text.Cache
2019
import androidx.compose.ui.text.ExperimentalTextApi
2120
import androidx.compose.ui.text.ExpireAfterAccessCache
2221
import androidx.compose.ui.text.InternalTextApi
@@ -296,7 +295,7 @@ internal class FontCache {
296295
internal val fonts = FontCollection()
297296
private val fontProvider = TypefaceFontProviderWithFallback()
298297
private val registered: MutableSet<String> = HashSet()
299-
private val typefacesCache: Cache<String, SkTypeface> = ExpireAfterAccessCache(
298+
private val typefacesCache = ExpireAfterAccessCache<String, SkTypeface>(
300299
60_000_000_000 // 1 minute
301300
)
302301

@@ -306,7 +305,7 @@ internal class FontCache {
306305
}
307306

308307
internal fun load(font: PlatformFont): FontLoadResult {
309-
val typeface = typefacesCache.get(font.cacheKey) {
308+
val typeface = typefacesCache.getOrPut(font.cacheKey) {
310309
loadTypeface(font)
311310
}
312311
ensureRegistered(typeface, font.cacheKey)

‎compose/ui/ui-text/src/skikoTest/kotlin/androidx/compose/ui/text/ExpireAfterAccessCacheTest.kt‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,45 +34,45 @@ class ExpireAfterAccessCacheTest {
3434

3535
@Test
3636
fun singleKey() {
37-
assertEquals("v1_1", cache.get("k1") { "v1_1" })
37+
assertEquals("v1_1", cache.getOrPut("k1") { "v1_1" })
3838
assertEquals(setOf("k1"), cache.map.keys)
3939
assertEquals(setOf("k1"), cache.accessTime.keys)
4040
assertEquals(0, cache.accessTime["k1"])
4141

4242
time += 10.millisToNanos()
4343

44-
assertEquals("v1_1", cache.get("k1") { "v1_2" })
44+
assertEquals("v1_1", cache.getOrPut("k1") { "v1_2" })
4545
assertEquals(setOf("k1"), cache.map.keys)
4646
assertEquals(setOf("k1"), cache.accessTime.keys)
4747
assertEquals(time, cache.accessTime["k1"])
4848

4949
time += 2.secondsToNanos()
5050

51-
assertEquals("v1_1", cache.get("k1") { "v1_3" })
51+
assertEquals("v1_1", cache.getOrPut("k1") { "v1_3" })
5252
assertEquals(setOf("k1"), cache.map.keys)
5353
assertEquals(setOf("k1"), cache.accessTime.keys)
5454
assertEquals(time, cache.accessTime["k1"])
5555
}
5656

5757
@Test
5858
fun manyKeys() {
59-
assertEquals("v1_1", cache.get("k1") { "v1_1" })
59+
assertEquals("v1_1", cache.getOrPut("k1") { "v1_1" })
6060
assertEquals(setOf("k1"), cache.map.keys)
6161
assertEquals(cache.map.keys, cache.accessTime.keys)
6262
assertEquals(0, cache.accessTime["k1"])
6363

6464
time += 10.millisToNanos()
6565

66-
assertEquals("v2_1", cache.get("k2") { "v2_1" })
66+
assertEquals("v2_1", cache.getOrPut("k2") { "v2_1" })
6767
assertEquals(setOf("k1", "k2"), cache.map.keys)
6868
assertEquals(cache.map.keys, cache.accessTime.keys)
6969
assertEquals(0, cache.accessTime["k1"])
7070
assertEquals(time, cache.accessTime["k2"])
7171

7272
time += 10.millisToNanos()
7373

74-
assertEquals("v2_1", cache.get("k2") { "v2_2" })
75-
assertEquals("v3_1", cache.get("k3") { "v3_1" })
74+
assertEquals("v2_1", cache.getOrPut("k2") { "v2_2" })
75+
assertEquals("v3_1", cache.getOrPut("k3") { "v3_1" })
7676
assertEquals(setOf("k1", "k2", "k3"), cache.map.keys)
7777
assertEquals(cache.map.keys, cache.accessTime.keys)
7878
assertEquals(0, cache.accessTime["k1"])
@@ -81,7 +81,7 @@ class ExpireAfterAccessCacheTest {
8181

8282
time += 2.secondsToNanos()
8383

84-
assertEquals("v2_1", cache.get("k2") { "v2_3" })
84+
assertEquals("v2_1", cache.getOrPut("k2") { "v2_3" })
8585
assertEquals(setOf("k2"), cache.map.keys)
8686
assertEquals(setOf("k2"), cache.accessTime.keys)
8787
assertEquals(time, cache.accessTime["k2"])

‎compose/ui/ui-text/src/webMain/kotlin/androidx/compose/ui/text/Cache.web.kt‎

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,82 @@
1616

1717
package androidx.compose.ui.text
1818

19-
// TODO Use WeakMap once available https://youtrack.jetbrains.com/issue/KT-44309
20-
internal actual typealias WeakKeysCache<K, V> = NoCache<K, V>
19+
import kotlin.js.ExperimentalWasmJsInterop
20+
import kotlin.js.JsAny
21+
import kotlin.js.JsReference
22+
import kotlin.js.get
23+
import kotlin.js.toJsReference
24+
import kotlin.js.unsafeCast
2125

22-
internal class NoCache<K : Any, V> : Cache<K, V> {
23-
override fun get(key: K, loader: (K) -> V): V = loader(key)
26+
// We can't use Js WeakMap https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap
27+
// because it uses identity equality for keys, but the usage of WeakKeysCache in SkikoParagraph requires that the
28+
// keys would have structural equality.
29+
@OptIn(ExperimentalWasmJsInterop::class)
30+
internal actual class WeakKeysCache<K : Any, V: Any> {
31+
32+
private val cache = HashMap<Key<K>, V>()
33+
34+
// When K is finalized, we'll eventually receive a callback.
35+
// And we use it to remove the Key(WeakReference<K>) from the HashMap.
36+
private val registry = FinalizationRegistry { keyJsReference ->
37+
val key: Key<K> = keyJsReference.unsafeCast<JsReference<Key<K>>>().get()
38+
cache.remove(key)
39+
}
40+
41+
actual inline fun getOrPut(key: K, loader: (K) -> V): V {
42+
// Here we don't run a loop over the cache keys to clear the unavailable (after GC) keys.
43+
// The loop would be not ignorably cheap, because it would make a js-interop call for deref on
44+
// every iteration - a concern for scrolls with different text items (getOrPut is called often).
45+
// Here we rely on FinalizationRegistry to clean the unavailable (after GC) keys.
46+
val weakKey = Key(key)
47+
48+
val existing = cache[weakKey]
49+
if (existing != null) return existing
50+
51+
val value = loader(key)
52+
cache[weakKey] = value
53+
54+
registry.register(key.toJsReference(), weakKey.toJsReference())
55+
56+
return value
57+
}
58+
59+
internal class Key<K : Any>(key: K) {
60+
private val ref = WeakReference(key)
61+
private val hash: Int = key.hashCode()
62+
63+
override fun equals(other: Any?): Boolean {
64+
if (this === other) return true
65+
if (other == null) return false
66+
other as Key<*>
67+
val a = ref.get()
68+
val b = other.ref.get()
69+
if (a == null || b == null) {
70+
// If either side is cleared, they should not be considered equal
71+
return false
72+
}
73+
return a == b
74+
}
75+
76+
override fun hashCode(): Int = hash
77+
}
78+
}
79+
80+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef
81+
@OptIn(ExperimentalWasmJsInterop::class)
82+
private external class WeakRef {
83+
constructor(target: JsAny)
84+
fun deref(): JsAny?
2485
}
86+
87+
@OptIn(ExperimentalWasmJsInterop::class)
88+
private class WeakReference<T : Any>(reference: T) {
89+
private var weakRef: WeakRef? = WeakRef(reference.toJsReference())
90+
fun get(): T? = weakRef?.deref()?.unsafeCast<JsReference<T>>()?.get()
91+
}
92+
93+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry
94+
@OptIn(ExperimentalWasmJsInterop::class)
95+
internal external class FinalizationRegistry(cleanup: (JsAny) -> Unit) {
96+
fun register(target: JsAny, heldValue: JsAny)
97+
}

0 commit comments

Comments
 (0)