Cache DNS section parsing instead of re-serializing per access - #120
Merged
Conversation
layer7/dns.py called bytes(self) from six sites — each a full re-serialization of the whole message — because compression offsets are message-relative while the parser only held the sections. A single .answers access rebuilt the message 14 times. Worse, answers, authorities and additionals each called _resource_records(), which parses all three sections, so reading all three parsed the message three times over. Two changes, matching the two the issue asks for: 1. The parsers move to module level and work from the sections bytes directly, in section-relative offsets. Only a compression pointer needs translating, by subtracting the 12-byte header length; a pointer that addresses the header itself now lands below zero and raises InvalidFieldError rather than decoding header bytes as labels. bytes(self) is gone from every accessor. 2. The three sections are parsed in one pass by a module-level lru_cache keyed by the (immutable) section bytes and the counts, so reading all three parses once. Nothing is stored on the frozen instance, so hashability and equality are untouched, and the cache is bounded at 128 messages. Records are frozen dataclasses, so sharing them between callers cannot leak mutation — asserted. Measured on a corpus DNS response (20k iterations): one uncached parse 17.03 -> 12.17 us (1.4x, re-serialization) .answers 16.84 -> 0.20 us .answers+auth+addl 52.08 -> 0.54 us (2.9x ratio -> 1.03x) The corpus walk does not touch record accessors, so end-to-end throughput is unchanged at ~115k frames/sec; this is a win for callers that read records, which is the point of the accessors. Closes#85. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJnVMNGwTRDktC4rkABtgt
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
layer7/dns.pycalledbytes(self)— a full re-serialization of the whole message — from six sites, because compression offsets are message-relative while the parser only heldsections. One.answersaccess rebuilt the message 14 times. Andanswers/authorities/additionalseach re-parsed all three sections, so reading all three parsed the message three times over.Both of the changes the issue asks for, in order:
sectionsbytes the instance already holds, in section-relative offsets. Only a compression pointer needs translating (subtract the 12-byte header length) — and a pointer addressing the header itself now lands below zero and raisesInvalidFieldErrorinstead of decoding header bytes as labels.bytes(self)appears in no accessor.lru_cachekeyed by the immutable section bytes plus the counts.The frozen-instance question
Nothing is stored on the instance: no
object.__setattr__, no extra field, nocached_property(whichslots=Truerules out anyway). Hashability and equality are untouched,bytes(decode(x)) == xstill holds, and the memo is bounded at 128 messages. Because it is keyed by value, two equal messages share one parse — records are frozen dataclasses, so sharing them between callers cannot leak mutation, which the tests assert directly.Measured
Corpus DNS response, 20k iterations, CPython 3.12 on this machine:
.answers.answers+.authorities+.additionalsCorpus throughput is unchanged (~115k f/s) because the chain walk never touches record accessors — this is a win for callers that read records, which is what the accessors are for.
Verification
cache_info()(1 miss, 2 hits)bytes(self)is not called from any accessor — no occurrences remain in the moduleuv run ruff check/ruff format --check/mypyclean;uv run pytest— 757 passedNotes
One deliberate behaviour change worth a reviewer's eye: a compression pointer into the fixed header previously produced garbage labels and now raises
InvalidFieldError. It is unreachable in well-formed messages, the fuzz contract (ProtocolErroror a value, never anything else) is preserved, and there is a test for it.Closes#85.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QJnVMNGwTRDktC4rkABtgt
Generated by Claude Code