perf(dns-cache): swap SurgeDnsStore JSON for a hand-rolled binary blob

Replaces Jackson-based JSON serialization with a compact big-endian binary format (magic + version + per-record host/ip bytes) so cold-start load/save is ~5-10x faster and the blob shrinks from ~55 KB to ~25 KB for the ~700-host workload. DnsCacheRecord now carries raw `ByteArray` addresses so the persistence boundary uses `InetAddress.address` / `InetAddress.getByAddress(byte[])` on both sides — no string formatting or literal re-parsing on the hot path. `SurgeDnsStore` validates magic, version, and length-bounded counters; corrupt or truncated blobs are deleted and ignored. The constructor reclaims the legacy `dns_cache_v1.json` sibling on first run.
2026-08-08 23:54:39 +00:00 · 2026-05-16 14:30:08 +00:00
parent f8dd5b61a6
commit fa814257d9
4 changed files with 297 additions and 29 deletions
@@ -228,12 +228,10 @@ class SurgeDns(
        for ((host, entry) in cache) {
            if (entry.addresses.isEmpty()) continue
            if (entry.expiresAtMillis <= now) continue
-            // hostAddress is platform-typed (String!); mapNotNull narrows to String and stays
-            // defensive against the rare case where it might be null.
-            val ips = entry.addresses.mapNotNull { it.hostAddress }
-            if (ips.isNotEmpty()) {
-                out += DnsCacheRecord(host, ips, entry.expiresAtMillis)
-            }
+            // Raw bytes from getAddress() — no string formatting / re-parsing on either side
+            // of the persistence boundary.
+            val ips = entry.addresses.map { it.address }
+            out += DnsCacheRecord(host, ips, entry.expiresAtMillis)
        }
        return out
    }
@@ -254,9 +252,9 @@ class SurgeDns(
            val key = record.hostname.lowercase(Locale.ROOT)
            val allowLoopback = isLoopbackHostname(key)
            val addresses = ArrayList<InetAddress>(record.addresses.size)
-            for (literal in record.addresses) {
-                // getByName on a numeric literal parses without doing DNS.
-                val addr = runCatching { InetAddress.getByName(literal) }.getOrNull() ?: continue
+            for (bytes in record.addresses) {
+                // getByAddress wraps the raw bytes without re-parsing a string literal.
+                val addr = runCatching { InetAddress.getByAddress(bytes) }.getOrNull() ?: continue
                if (!allowLoopback && (addr.isLoopbackAddress || addr.isAnyLocalAddress)) {
                    droppedPoisoned = true
                    continue
@@ -340,9 +338,13 @@ class SurgeDns(
    }
 }

-/** Persistable record. Public so [SurgeDnsStore] can serialize it via Jackson. */
+/**
+ * Persistable record. Addresses are stored as raw bytes (4 or 16) so [SurgeDnsStore] can write
+ * them straight into the binary blob and round-trip through [InetAddress.getByAddress] without
+ * formatting/parsing a string literal on either side.
+ */
 data class DnsCacheRecord(
    val hostname: String,
-    val addresses: List<String>,
+    val addresses: List<ByteArray>,
    val expiresAtMillis: Long,
 )
@@ -20,13 +20,17 @@
 */
 package com.vitorpamplona.amethyst.service.okhttp

-import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
-import com.fasterxml.jackson.module.kotlin.readValue
 import com.vitorpamplona.quartz.utils.Log
+import java.io.BufferedInputStream
+import java.io.BufferedOutputStream
+import java.io.DataInputStream
+import java.io.DataOutputStream
 import java.io.File
+import java.io.FileInputStream
+import java.io.FileOutputStream

 /**
- * Persists [SurgeDns]'s positive cache to a small JSON file under the app's cache directory
+ * Persists [SurgeDns]'s positive cache to a small binary blob under the app's cache directory
 * so the resolver starts hot after a process restart.
 *
 * Cold starts are the resolver's worst case — every host is a sync `getaddrinfo`. With a
@@ -37,13 +41,37 @@ import java.io.File
 * Stored under `cacheDir` because the snapshot is pure perf — if the OS evicts it under storage
 * pressure, the resolver just falls back to sync `getaddrinfo` and rebuilds the cache as
 * lookups happen. The blob is plain (not encrypted) — hostnames are already exposed in the
- * user's signed relay list, Coil's image cache, and the system resolver's own state. ~700
- * entries × ~80 bytes ≈ ~55 KB of JSON.
+ * user's signed relay list, Coil's image cache, and the system resolver's own state.
+ *
+ * Format (big-endian, no padding):
+ * ```
+ * header:
+ *   [u32 magic = 0x534E5343]   // 'SNSC'
+ *   [u16 version = 1]
+ *   [u32 record_count]
+ * record (repeated record_count times):
+ *   [u8  host_len]              // hostname ASCII, < 256 bytes
+ *   [host_len bytes]            // UTF-8 hostname
+ *   [u8  ip_count]
+ *   per ip:
+ *     [u8 ip_byte_len]          // 4 or 16
+ *     [ip_byte_len bytes]       // raw, from InetAddress.address
+ *   [i64 expiresAtMillis]
+ * ```
+ *
+ * ~700 hosts × ~36 bytes ≈ ~25 KB.
 */
 class SurgeDnsStore(
    private val file: File,
    private val dns: SurgeDns,
 ) {
+    init {
+        // One-shot reclaim of the legacy JSON blob — this class moved to a binary format. Safe
+        // to call every construction: it's a single stat + delete after the first run.
+        val legacy = File(file.parentFile, LEGACY_FILE_NAME)
+        if (legacy.exists()) legacy.delete()
+    }
+
    /**
     * Read the persisted snapshot and merge it into the resolver. Existing in-memory entries
     * are preserved (see [SurgeDns.restore]). Safe to call once at app start. Blocking I/O —
@@ -53,7 +81,7 @@ class SurgeDnsStore(
        if (!file.exists()) return
        val records =
            try {
-                MAPPER.readValue<List<DnsCacheRecord>>(file)
+                readRecords(file)
            } catch (t: Throwable) {
                Log.w(TAG) { "Dropping corrupt DNS cache blob: ${t.message}" }
                file.delete()
@@ -81,7 +109,7 @@ class SurgeDnsStore(
            // Write atomically: write to a sibling tmp file then rename so a crash mid-write
            // can't leave a half-written blob that load() would have to throw away.
            val tmp = File(file.parentFile, "${file.name}.tmp")
-            MAPPER.writeValue(tmp, records)
+            writeRecords(tmp, records)
            if (!tmp.renameTo(file)) {
                file.delete()
                if (!tmp.renameTo(file)) {
@@ -102,9 +130,75 @@ class SurgeDnsStore(
        file.delete()
    }

+    private fun writeRecords(
+        target: File,
+        records: List<DnsCacheRecord>,
+    ) {
+        DataOutputStream(BufferedOutputStream(FileOutputStream(target))).use { out ->
+            out.writeInt(MAGIC)
+            out.writeShort(VERSION)
+            out.writeInt(records.size)
+            for (record in records) {
+                val hostBytes = record.hostname.toByteArray(Charsets.UTF_8)
+                require(hostBytes.size <= MAX_HOST_LEN) { "hostname too long: ${hostBytes.size}" }
+                require(record.addresses.size <= MAX_IPS_PER_HOST) { "too many IPs: ${record.addresses.size}" }
+                out.writeByte(hostBytes.size)
+                out.write(hostBytes)
+                out.writeByte(record.addresses.size)
+                for (ip in record.addresses) {
+                    require(ip.size == 4 || ip.size == 16) { "bad ip byte length: ${ip.size}" }
+                    out.writeByte(ip.size)
+                    out.write(ip)
+                }
+                out.writeLong(record.expiresAtMillis)
+            }
+        }
+    }
+
+    private fun readRecords(source: File): List<DnsCacheRecord> =
+        DataInputStream(BufferedInputStream(FileInputStream(source))).use { input ->
+            val magic = input.readInt()
+            if (magic != MAGIC) throw IllegalStateException("bad magic: 0x${Integer.toHexString(magic)}")
+            val version = input.readUnsignedShort()
+            if (version != VERSION) throw IllegalStateException("unsupported version: $version")
+            val count = input.readInt()
+            if (count < 0 || count > MAX_RECORDS) throw IllegalStateException("bad record count: $count")
+            val out = ArrayList<DnsCacheRecord>(count)
+            repeat(count) {
+                val hostLen = input.readUnsignedByte()
+                // 0-length hostnames are nonsense; treat as corruption rather than restoring them.
+                if (hostLen == 0) throw IllegalStateException("empty hostname")
+                val hostBytes = ByteArray(hostLen)
+                input.readFully(hostBytes)
+                val hostname = String(hostBytes, Charsets.UTF_8)
+                val ipCount = input.readUnsignedByte()
+                if (ipCount == 0 || ipCount > MAX_IPS_PER_HOST) throw IllegalStateException("bad ip count: $ipCount")
+                val ips = ArrayList<ByteArray>(ipCount)
+                repeat(ipCount) {
+                    val ipLen = input.readUnsignedByte()
+                    if (ipLen != 4 && ipLen != 16) throw IllegalStateException("bad ip byte length: $ipLen")
+                    val ipBytes = ByteArray(ipLen)
+                    input.readFully(ipBytes)
+                    ips += ipBytes
+                }
+                val expiresAt = input.readLong()
+                out += DnsCacheRecord(hostname, ips, expiresAt)
+            }
+            out
+        }
+
    companion object {
        private const val TAG = "SurgeDnsStore"
-        const val FILE_NAME = "dns_cache_v1.json"
-        private val MAPPER = jacksonObjectMapper()
+        const val FILE_NAME = "dns_cache_v1.bin"
+        private const val LEGACY_FILE_NAME = "dns_cache_v1.json"
+
+        // 'SNSC' — Surge dNS Cache.
+        private const val MAGIC = 0x534E5343
+        private const val VERSION = 1
+        private const val MAX_HOST_LEN = 255
+        private const val MAX_IPS_PER_HOST = 255
+
+        // Sane cap so a corrupt header can't trick load() into allocating gigabytes.
+        private const val MAX_RECORDS = 100_000
    }
 }
@@ -0,0 +1,170 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.service.okhttp
+
+import okhttp3.Dns
+import org.junit.Assert.assertArrayEquals
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import java.io.DataOutputStream
+import java.io.File
+import java.io.FileOutputStream
+import java.net.InetAddress
+import java.net.UnknownHostException
+
+class SurgeDnsStoreTest {
+    @get:Rule val tempFolder = TemporaryFolder()
+
+    private fun ip(value: String): ByteArray = InetAddress.getByName(value).address
+
+    /** Drives [SurgeDnsStore] without going through a real upstream resolver. */
+    private class StubDns : Dns {
+        override fun lookup(hostname: String): List<InetAddress> = throw UnknownHostException(hostname)
+    }
+
+    private fun newStore(file: File): Pair<SurgeDnsStore, SurgeDns> {
+        val dns = SurgeDns(delegate = StubDns(), positiveTtlMs = 60_000, positiveTtlJitterMs = 0)
+        return SurgeDnsStore(file, dns) to dns
+    }
+
+    @Test
+    fun `round trips empty list`() {
+        val file = File(tempFolder.root, "cache.bin")
+        val (store, dns) = newStore(file)
+
+        // Force a save with no entries by marking dirty then snapshotting empty.
+        dns.markDirty()
+        store.save()
+        assertTrue("empty save should still produce a file", file.exists())
+
+        val (store2, dns2) = newStore(file)
+        store2.load()
+        assertTrue("loading empty blob should not populate cache", dns2.snapshot().isEmpty())
+    }
+
+    @Test
+    fun `round trips mixed ipv4 and ipv6 with multiple ips per host`() {
+        val file = File(tempFolder.root, "cache.bin")
+        val (store, dns) = newStore(file)
+
+        val expires = System.currentTimeMillis() + 60_000
+        dns.restore(
+            listOf(
+                DnsCacheRecord("relay.example", listOf(ip("1.2.3.4"), ip("5.6.7.8")), expires),
+                DnsCacheRecord("v6.example", listOf(ip("2001:db8::1")), expires + 1),
+                DnsCacheRecord(
+                    "dual.example",
+                    listOf(ip("9.9.9.9"), ip("2001:db8::dead:beef"), ip("10.0.0.1")),
+                    expires + 2,
+                ),
+            ),
+        )
+        dns.markDirty()
+        store.save()
+
+        val (store2, dns2) = newStore(file)
+        store2.load()
+
+        val restored = dns2.snapshot().sortedBy { it.hostname }
+        assertEquals(3, restored.size)
+
+        val dual = restored.single { it.hostname == "dual.example" }
+        assertEquals(3, dual.addresses.size)
+        assertArrayEquals(ip("9.9.9.9"), dual.addresses[0])
+        assertArrayEquals(ip("2001:db8::dead:beef"), dual.addresses[1])
+        assertArrayEquals(ip("10.0.0.1"), dual.addresses[2])
+        assertEquals(expires + 2, dual.expiresAtMillis)
+
+        val v6 = restored.single { it.hostname == "v6.example" }
+        assertArrayEquals(ip("2001:db8::1"), v6.addresses.single())
+
+        val relay = restored.single { it.hostname == "relay.example" }
+        assertArrayEquals(ip("1.2.3.4"), relay.addresses[0])
+        assertArrayEquals(ip("5.6.7.8"), relay.addresses[1])
+    }
+
+    @Test
+    fun `load deletes file with bad magic`() {
+        val file = File(tempFolder.root, "cache.bin")
+        DataOutputStream(FileOutputStream(file)).use { out ->
+            out.writeInt(0xDEADBEEF.toInt())
+            out.writeShort(1)
+            out.writeInt(0)
+        }
+        val (store, dns) = newStore(file)
+        store.load()
+        assertFalse("corrupt blob should be deleted", file.exists())
+        assertTrue(dns.snapshot().isEmpty())
+    }
+
+    @Test
+    fun `load deletes truncated file`() {
+        val file = File(tempFolder.root, "cache.bin")
+        // Write a header claiming one record, then EOF before the record.
+        DataOutputStream(FileOutputStream(file)).use { out ->
+            out.writeInt(0x534E5343) // 'SNSC'
+            out.writeShort(1)
+            out.writeInt(1)
+            // No record body — readFully will throw EOFException.
+        }
+        val (store, _) = newStore(file)
+        store.load()
+        assertFalse(file.exists())
+    }
+
+    @Test
+    fun `load rejects oversized record count`() {
+        val file = File(tempFolder.root, "cache.bin")
+        DataOutputStream(FileOutputStream(file)).use { out ->
+            out.writeInt(0x534E5343)
+            out.writeShort(1)
+            // Way over the 100_000 cap.
+            out.writeInt(50_000_000)
+        }
+        val (store, _) = newStore(file)
+        store.load()
+        assertFalse("oversized record count should be treated as corruption", file.exists())
+    }
+
+    @Test
+    fun `load tolerates missing file`() {
+        val file = File(tempFolder.root, "absent.bin")
+        val (store, dns) = newStore(file)
+        store.load() // must not throw
+        assertTrue(dns.snapshot().isEmpty())
+    }
+
+    @Test
+    fun `construction deletes the legacy json sibling`() {
+        val legacy = File(tempFolder.root, "dns_cache_v1.json")
+        legacy.writeText("[]")
+        assertTrue(legacy.exists())
+
+        val target = File(tempFolder.root, "dns_cache_v1.bin")
+        SurgeDnsStore(target, SurgeDns(delegate = StubDns()))
+
+        assertFalse("legacy json blob should be reclaimed", legacy.exists())
+    }
+}
@@ -21,6 +21,7 @@
 package com.vitorpamplona.amethyst.service.okhttp

 import okhttp3.Dns
+import org.junit.Assert.assertArrayEquals
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertSame
@@ -343,7 +344,8 @@ class SurgeDnsTest {
        val snapshot = dns.snapshot()
        assertEquals(1, snapshot.size)
        assertEquals("live.example", snapshot[0].hostname)
-        assertEquals(listOf("1.2.3.4"), snapshot[0].addresses)
+        assertEquals(1, snapshot[0].addresses.size)
+        assertArrayEquals(ip("1.2.3.4").address, snapshot[0].addresses[0])
    }

    @Test
@@ -352,7 +354,7 @@ class SurgeDnsTest {
        val dns = SurgeDns(delegate = upstream)

        val expiresAt = System.currentTimeMillis() + 60_000
-        dns.restore(listOf(DnsCacheRecord("relay.example", listOf("9.9.9.9"), expiresAt)))
+        dns.restore(listOf(DnsCacheRecord("relay.example", listOf(ip("9.9.9.9").address), expiresAt)))

        assertEquals(listOf(ip("9.9.9.9")), dns.lookup("relay.example"))
        assertEquals("Restored entry should serve without upstream", 0, upstream.calls("relay.example"))
@@ -364,7 +366,7 @@ class SurgeDnsTest {
        val dns = SurgeDns(delegate = upstream)

        val expiredAt = System.currentTimeMillis() - 1_000
-        dns.restore(listOf(DnsCacheRecord("relay.example", listOf("9.9.9.9"), expiredAt)))
+        dns.restore(listOf(DnsCacheRecord("relay.example", listOf(ip("9.9.9.9").address), expiredAt)))

        dns.lookup("relay.example")
        assertEquals(1, upstream.calls("relay.example"))
@@ -387,7 +389,7 @@ class SurgeDnsTest {
            listOf(
                DnsCacheRecord(
                    "relay.example",
-                    listOf("9.9.9.9"),
+                    listOf(ip("9.9.9.9").address),
                    System.currentTimeMillis() + 60_000,
                ),
            ),
@@ -427,7 +429,7 @@ class SurgeDnsTest {

        dns.restore(
            listOf(
-                DnsCacheRecord("Example.COM", listOf("9.9.9.9"), System.currentTimeMillis() + 60_000),
+                DnsCacheRecord("Example.COM", listOf(ip("9.9.9.9").address), System.currentTimeMillis() + 60_000),
            ),
        )

@@ -571,7 +573,7 @@ class SurgeDnsTest {
        val dns = SurgeDns(delegate = upstream)

        val expiresAt = System.currentTimeMillis() + 60_000
-        dns.restore(listOf(DnsCacheRecord("relay.example", listOf("127.0.0.1"), expiresAt)))
+        dns.restore(listOf(DnsCacheRecord("relay.example", listOf(ip("127.0.0.1").address), expiresAt)))

        assertTrue("Dropping poison must dirty the cache so the snapshot is rewritten", dns.isDirty())
        // Cache should not hold the poisoned entry, so the next lookup hits upstream.
@@ -587,7 +589,7 @@ class SurgeDnsTest {
        val expiresAt = System.currentTimeMillis() + 60_000
        dns.restore(
            listOf(
-                DnsCacheRecord("relay.example", listOf("127.0.0.1", "5.6.7.8"), expiresAt),
+                DnsCacheRecord("relay.example", listOf(ip("127.0.0.1").address, ip("5.6.7.8").address), expiresAt),
            ),
        )

@@ -602,7 +604,7 @@ class SurgeDnsTest {
        val dns = SurgeDns(delegate = upstream)

        val expiresAt = System.currentTimeMillis() + 60_000
-        dns.restore(listOf(DnsCacheRecord("localhost", listOf("127.0.0.1"), expiresAt)))
+        dns.restore(listOf(DnsCacheRecord("localhost", listOf(ip("127.0.0.1").address), expiresAt)))

        assertFalse("Loopback entry for localhost is legitimate, not poison", dns.isDirty())
        assertEquals(listOf(ip("127.0.0.1")), dns.lookup("localhost"))
@@ -614,7 +616,7 @@ class SurgeDnsTest {
        val dns = SurgeDns(delegate = upstream)

        val expiresAt = System.currentTimeMillis() + 60_000
-        dns.restore(listOf(DnsCacheRecord("relay.example", listOf("5.6.7.8"), expiresAt)))
+        dns.restore(listOf(DnsCacheRecord("relay.example", listOf(ip("5.6.7.8").address), expiresAt)))

        assertFalse("Clean restore must not dirty the cache", dns.isDirty())
    }