diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt index c0c7c222d152..647e68e8e690 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt @@ -18,7 +18,6 @@ package okhttp3.internal.dns import java.io.IOException -import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicReference import kotlin.time.ComparableTimeMark as Time import kotlin.time.Duration @@ -29,12 +28,13 @@ import okhttp3.internal.OkHttpInternalApi import okhttp3.internal.concurrent.TaskRunner import okhttp3.internal.dns.StateMachineDnsCall.Transport -// TODO: evict old entries from cache using State.lastRequestedAt - /** * A DNS transport that caches responses according to their [ResourceRecord.timeToLive], bounded by * a user-supplied minimum and maximum cache duration. * + * Cache Hits + * ---------- + * * The age of the result impacts how queries are satisfied: * * * After [Result.expireAt], the cached result is not used and a call to the underlying transport @@ -51,6 +51,18 @@ import okhttp3.internal.dns.StateMachineDnsCall.Transport * * If this receives multiple equivalent queries, it combines them into a single query on the * underlying transport. + * + * Memory Usage + * ------------ + * + * By default, this retains the 1,000 most recently accessed entries. Most hostnames will require 3 + * entries (`TYPE_A`, `TYPE_AAAA`, and `TYPE_HTTPS`). + * + * Between evictions the memory cache will grow to double that max, 2,000 entries. + * + * Each entry consumes about 400 bytes of memory. + * + * In total, the default cache will use about 800 KiB of memory. */ @OkHttpInternalApi @OptIn(ExperimentalTime::class) // We know Clock and Instant will be stable in Kotlin 2.3. @@ -62,8 +74,28 @@ class CachingTransport( private val maximumTimeToLive: Duration = 300.seconds, private val failureTimeToLive: Duration = 10.seconds, private val revalidateBeforeExpire: Duration = 5.seconds, + maxEntryCount: Int = 1000, ) : Transport> { - private val entries = ConcurrentHashMap() + private val cache = + object : MemoryCache( + timeSource = timeSource, + maxSize = maxEntryCount, + ) { + override fun lastRequestedAt( + now: Time, + value: CachingTransport.Entry, + ): Time? { + val state = value.state.get() + + // If it's already expired, evict immediately. + if (state.inFlightCall == null) { + val expireAt = state.result?.expireAt ?: return null + if (expireAt <= now) return null + } + + return state.lastRequestedAt + } + } init { require(failureTimeToLive >= 0.seconds) @@ -73,8 +105,7 @@ class CachingTransport( } override fun newQuery(question: Question): Query { - val inserted = Entry(question) - val entry = entries.putIfAbsent(question, inserted) ?: inserted + val entry = cache.computeIfAbsent(question) { Entry(question) } return Query(entry) } diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/MemoryCache.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/MemoryCache.kt new file mode 100644 index 000000000000..972fc0432957 --- /dev/null +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/MemoryCache.kt @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2026 OkHttp Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package okhttp3.internal.dns + +import java.util.PriorityQueue +import java.util.concurrent.ConcurrentHashMap +import kotlin.time.ComparableTimeMark as Time +import kotlin.time.TimeSource + +/** + * This cache is configured with a target [maxSize], but it will not evict entries until it + * has twice as many entries. + * + * This retains entries in least-recently-used order. + * + * This evicts in big batches because each eviction must traverse the entire cache. + */ +abstract class MemoryCache( + private val timeSource: TimeSource.WithComparableMarks, + val maxSize: Int, +) { + private val entries = ConcurrentHashMap() + + init { + require(maxSize >= 0) + } + + /** + * Returns the time this value was most recently used, in order to make an eviction decision. This + * should return null if this element should be evicted immediately. + */ + abstract fun lastRequestedAt( + now: Time, + value: V, + ): Time? + + /** + * Similar to [ConcurrentHashMap.computeIfAbsent], but this will also prune the cache to size if + * this function grows the cache to double [maxSize]. + */ + fun computeIfAbsent( + key: K, + computeValue: () -> V, + ): V { + if (maxSize == 0) return computeValue() + + val result = entries[key] + if (result != null) return result + + val created = computeValue() + val existing = entries.putIfAbsent(key, created) + if (existing != null) return existing + + // If there's more than 2x as many entries as the max, automatically evict. + val toEvict = entries.size - maxSize + if (toEvict >= maxSize) { + evict(toEvict) + } + + return created + } + + /** + * Manually evict [count] entries from this cache. + * + * If the cache is smaller than [count], this evicts everything. + * + * Entries that are expired are evicted unconditionally, even if this causes more than [count] + * entries to be evicted. If [count] or more expired entries are evicted, non-expired entries will + * not be evicted. + */ + fun evict(count: Int) { + // Note that we call lastRequestedAt() exactly once for each entry. If we called it on-demand in + // the comparator, we'd break the PriorityQueue's invariant that the sort order is stable. + var count = count + val now = timeSource.markNow() + val entriesToEvict = PriorityQueue>>(count + 1, NewestFirst) + val i = entries.entries.iterator() + while (i.hasNext()) { + val entry = i.next() + + when (val lastRequestedAt = lastRequestedAt(now, entry.value)) { + // It's expired, evict unconditionally. + null -> { + i.remove() + count-- + } + + // It's not expired. Make it a candidate for eviction. + else -> { + entriesToEvict.add(lastRequestedAt to entry) + } + } + + // If we're above our target eviction count, save the newest entry from eviction. + if (entriesToEvict.size > count) { + entriesToEvict.poll() + } + } + + for ((_, entry) in entriesToEvict) { + entries.remove(entry.key, entry.value) + } + } + + private object NewestFirst : Comparator> { + override fun compare( + o1: Pair, + o2: Pair, + ) = o2.first.compareTo(o1.first) + } +} diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/MemoryCacheTest.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/MemoryCacheTest.kt new file mode 100644 index 000000000000..be719c1771b9 --- /dev/null +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/MemoryCacheTest.kt @@ -0,0 +1,240 @@ +/* + * Copyright (c) 2026 OkHttp Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package okhttp3.internal.dns + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isNull +import assertk.assertions.isSameInstanceAs +import kotlin.test.Test +import kotlin.time.AbstractLongTimeSource +import kotlin.time.ComparableTimeMark as Time +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds +import kotlin.time.DurationUnit + +class MemoryCacheTest { + private var time = 0.seconds + private val timeSource = + object : AbstractLongTimeSource(DurationUnit.SECONDS) { + override fun read() = time.inWholeSeconds + } + + private val cache = + object : MemoryCache( + timeSource = timeSource, + maxSize = 4, + ) { + override fun lastRequestedAt( + now: Time, + value: Record, + ): Time? { + val evictAt = value.evictAt ?: return null + if (now >= evictAt) return null + return value.lastRequestedAt + } + } + + @Test + fun `computeIfAbsent remembers values`() { + val a0 = cache.access("a") + val b0 = cache.access("b") + val c0 = cache.access("c") + + cache.assertPresent("a", a0) + cache.assertPresent("b", b0) + cache.assertPresent("c", c0) + } + + /** Use a re-entrant call to simulate a concurrent call.*/ + @Test + fun `computeIfAbsent race`() { + val now = timeSource.markNow() + val r0 = Record("r0", now, now) + val r1 = Record("r1", now, now) + val a0 = + cache.computeIfAbsent("a") { + val a1 = cache.computeIfAbsent("a") { r1 } + assertThat(a1).isSameInstanceAs(r1) + r0 + } + assertThat(a0).isSameInstanceAs(r1) + } + + @Test + fun `evict removes expired records`() { + cache.access("b") + sleep(7.seconds) + val a0 = cache.access("a") + val c0 = cache.access("c") + + // All elements are in the cache and have up-to-date lastRequestedAt times. + sleep(1.seconds) + cache.access("a") + sleep(1.seconds) + cache.access("b") + sleep(1.seconds) + cache.access("c") + + // Evict removes b, which has reached its evictAt time. + cache.evict(1) + cache.assertPresent("a", a0) + cache.assertAbsent("b") + cache.assertPresent("c", c0) + } + + @Test + fun `evict removes all expired records`() { + cache.access("a") + cache.access("b") + cache.access("c") + sleep(10.seconds) + + cache.evict(1) + cache.assertAbsent("a") + cache.assertAbsent("b") + cache.assertAbsent("c") + } + + @Test + fun `evict removes least recently used element`() { + val a0 = cache.access("a") + cache.access("b") + val c0 = cache.access("c") + + sleep(1.seconds) + cache.access("b") + sleep(1.seconds) + cache.access("a") + sleep(1.seconds) + cache.access("c") + + cache.evict(1) + cache.assertPresent("a", a0) + cache.assertAbsent("b") + cache.assertPresent("c", c0) + } + + @Test + fun `evict removes expired element and least recently used element`() { + // Make 'd' expired. + cache.access("d") + sleep(10.seconds) + val a0 = cache.access("a") + cache.access("b") + val c0 = cache.access("c") + val e0 = cache.access("e") + + // Make 'b' the least recently used. + sleep(1.seconds) + cache.access("b") + sleep(1.seconds) + cache.access("a") + sleep(1.seconds) + cache.access("c") + sleep(1.seconds) + cache.access("d") + sleep(1.seconds) + cache.access("e") + + cache.evict(2) + cache.assertPresent("a", a0) + cache.assertAbsent("b") // Least recently used. + cache.assertPresent("c", c0) + cache.assertAbsent("d") // Expired. + cache.assertPresent("e", e0) + } + + /** + * The test's configured with a max size of 4, so adding the 8th element should automatically + * resize it to 4 elements. + */ + @Test + fun `automatic eviction when size is double max`() { + sleep(1.seconds) + val a0 = cache.access("a") + sleep(1.seconds) + val b0 = cache.access("b") + sleep(1.seconds) + val c0 = cache.access("c") + sleep(1.seconds) + val d0 = cache.access("d") + sleep(1.seconds) + val e0 = cache.access("e") + sleep(1.seconds) + val f0 = cache.access("f") + sleep(1.seconds) + val g0 = cache.access("g") + + // Nothing evicted after 7 inserts. + cache.assertPresent("a", a0) + cache.assertPresent("b", b0) + cache.assertPresent("c", c0) + cache.assertPresent("d", d0) + cache.assertPresent("e", e0) + cache.assertPresent("f", f0) + cache.assertPresent("g", g0) + + // After the 8th insert, it evicts down to size 4. + sleep(1.seconds) + val h0 = cache.access("h") + cache.assertAbsent("a") + cache.assertAbsent("b") + cache.assertAbsent("c") + cache.assertAbsent("d") + cache.assertPresent("e", e0) + cache.assertPresent("f", f0) + cache.assertPresent("g", g0) + cache.assertPresent("h", h0) + } + + class Record( + val name: String, + var lastRequestedAt: Time?, + var evictAt: Time?, + ) { + override fun toString() = name + } + + private fun sleep(duration: Duration) { + time += duration + } + + fun MemoryCache.access(key: String): Record { + val now = timeSource.markNow() + val result = computeIfAbsent(key) { Record(key, now, now + 10.seconds) } + result.lastRequestedAt = now + return result + } + + fun MemoryCache.assertAbsent(key: String) { + val actual = + try { + computeIfAbsent(key) { throw Exception("absent!") } + } catch (_: Exception) { + null + } + assertThat(actual).isNull() + } + + fun MemoryCache.assertPresent( + key: String, + expected: Record, + ) { + val actual = computeIfAbsent(key) { throw Exception("absent!") } + assertThat(actual).isEqualTo(expected) + } +} diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt index b185420737d9..a520b6173730 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt @@ -58,6 +58,7 @@ class StateMachineDnsCallTester internal constructor() { maximumTimeToLive = 60.seconds, failureTimeToLive = 5.seconds, revalidateBeforeExpire = 2.seconds, + maxEntryCount = 4, ) fun newCall(