Skip to content

perf: cut extraction time by ~46% on an HTTP capture, with byte-identical output - #420

Merged
JarryShaw merged 18 commits into
mainfrom
perf/hot-path-wins
Sep 17, 2026
Merged

JarryShaw merged 18 commits into
mainfrom
perf/hot-path-wins

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

A profiling pass before 1.5.0, per the ask to "see if there's anything to improve/accelerate the library". Four optimisations, one reverted, and a 643-line PROFILING-NOTES.md carrying the measurements — including the suspects that turned out not to be worth touching, which is the half that stops the next person re-treading it.

Measured, and independently re-measured

http.pcap (1117 frames), best-of-7, store=False, nofile=True, separate processes:

capture before after
http.pcap 1042.7 ms 560.7 ms −46.2%
test.pcap 28.3 ms 17.0 ms −39.8%
many_interfaces.pcapng 49.6 ms 39.2 ms −20.9%
profile.pcapng 43.9 ms 35.2 ms −19.7%
ipv6.pcap 5.06 ms 4.37 ms −13.6%
ipv4.pcap 1.90 ms 1.66 ms −12.6%

The headline row is my own A/B against origin/main on the same interpreter, which came out slightly better than the −44.8% originally reported.

Correctness first

All 14 committed captures, serialised to both tree and json, with reassembly and flow tracing on — 30 files, byte-for-byte identical to main. diff -rq clean. Full suite: 782 passed, 17 skipped, zero failures. pylint and mypy message sets identical to pristine.

Two things in the diff could have been subtly wrong, and both were checked rather than inferred from the passing tests:

  • FieldBase.__copy__ replaces the generic copy.copy path with __new__ plus a __dict__ update. That is only equivalent if nothing in corekit/fields/ defines __slots__, __getstate__, __setstate__, __reduce__ or __deepcopy__ — grep confirms none does.
  • Hoisting length = field.length above field.unpack(...) in Schema.unpack would skew the later consumed = length - field.option_padding arithmetic if unpack could change _length. It cannot: _length is assigned only in the constructor and in __call__ (which runs before the hoist), while unpack touches _option_padding only.

The four wins

  • fa0920f97 — charset detection was uncached, and was 30% of an HTTP extraction. 3011 chardet.detect() calls over 163 distinct bytestrings, 94.6% of them redundant: an HTTP-heavy capture asks for the encoding of b'Connection' once per message and gets the same answer every time. chardet.detect is a pure function of its bytes, so the verdict is now memoised on them, bounded at 1024 entries so a capture full of never-repeating text cannot retain all of it. Wired into both call sites — attribution mattered, since http.pcap only reaches ProtocolBase.decode and many_interfaces.pcapng only StringField.post_process. −30.3%.
  • 5ce03cd77 — FieldBase.__copy__, which helps every capture shape. Every field of every protocol is copied once per packet, 63207 times per extraction, and it was falling through __reduce_ex__/_reconstruct. Microbenchmark 1.66 µs → 0.43 µs. −11% to −16% everywhere.
  • 0c1387460 — Field.length read once instead of two to four times. It recomputes struct.calcsize() on every read: 176908 calls per extraction. −2%.
  • 0aca00690 — Schema.__setattr__ was re-entering itself once per field assigned. 180297 of 254043 calls were that round trip, each one missing the __fields__ test and falling through to object.__setattr__. −3.7%.

One optimisation reverted

faef5d649 gives back a measured −1.0%: hoisting an isinstance into a local cost mypy its type narrowing and added four attr-defined errors. Not a trade worth making in this codebase.

Three larger wins found and deliberately not taken

All outside the parse hot path, all wanting their own review:

  1. dumpkit/pcap.py:94,128 — the flow dumper reopens the output file per frame and rebuilds a whole Frame to get bytes it already holds. 80% of the flow-tracing cost (+849 ms → +168 ms in a counterfactual), with 330 of 331 output files byte-identical. Best-evidenced and lowest-risk change available; it is not here only because it is a separate concern from the field machinery.
  2. reassembly/ip.py:189 — analyze() runs eagerly on every frame, fragmented or not, because toolkit/pcap.py:53 filters only on DF. http.pcap contains zero actual fragments and still yields 1117 "datagrams". 86% of the IP-reassembly cost plus a 133 ms GC bill. Making packet lazy is mechanical; whether unfragmented frames should be emitted at all is a design call for you, not something to decide inside a perf pass.
  3. collections.py:262-270 parses every option twice — 2274 schema unpacks for 1137 options, the pre-parse always discarded. ~15% of a PCAP-NG extraction, higher risk than the rest.

Two non-performance defects found while profiling

Neither is fixed here; both will be filed:

  • schema_final's generated typed __init__ is dead code — the schema.py:77 guard is always False because Schema.__init__ is __update__, so __post_init__ never runs.
  • TCP(**kwargs) reliably raises AttributeError at tcp.py:479.

Dismissed with data, so nobody repeats it

logging.py (zero logger calls during an extraction; 0.03% in the heaviest shape — all lazy %s, the one f-string is in vendor/__main__.py), multidict.py (<1%), ProtoChain (0.3%), _import_next_layer (0.035 s tottime; its large cumtime is just recursion), Info/InfoMeta (0.0% of construction, 4.4% of parse) — the most-suspected component turned out not to be the problem — store=True (1.3%), and in.pcap as a profiling target at all (6 frames, setup dominates).

Two measurement traps worth knowing, both recorded in the notes: reassembly=True and trace=True are no-ops without ip=/tcp=, so a benchmark passing only the switch measures nothing; and timing several capture shapes in one process inflates them by up to 73%.

chardet.detect() is a pure function of the bytes it is given, and by far the
most expensive step in decoding a text field. Nothing cached it, so an
HTTP-heavy capture re-detected the encoding of every repeated header name and
value on every message: 3011 detect() calls over only 163 distinct bytestrings
on examples/captures/http.pcap, a 94.6% redundancy rate.

- add a bounded functools.lru_cache'd _detect_charset() helper in
  corekit/fields/strings.py, the module that already owns the chardet
  dependency, and use it from both call sites -- StringField.post_process and
  ProtocolBase.decode, which had the same expression written out twice.

Measured on examples/captures/http.pcap (1117 frames), best of 7 runs:
1041.5 ms -> 726.3 ms, -30.3%. test.pcap 28.3 -> 21.0 ms, -25.7%. Captures with
no text fields are unchanged. Serialising all 14 fixtures to both tree and json
form, with and without reassembly, gives byte-identical output (33 MB, 60 runs).
Schema.unpack() calls field(packet) once per field per packet, and that returns
copy.copy(self). With no __copy__ hook, copy.copy fell through to the generic
pickle-style path -- object.__reduce_ex__(4), copyreg.__newobj__, then
copy._reconstruct -- which on examples/captures/http.pcap ran 63207 times per
extraction and cost four separate Python frames per copy.

- add FieldBase.__copy__ doing exactly what _reconstruct would have done for an
  object with a plain __dict__ and no __getstate__/__setstate__: cls.__new__(cls)
  followed by a shallow __dict__.update. No field class defines __new__,
  __slots__, __reduce__ or the getstate/setstate pair, so the two paths are
  equivalent by construction; a microbenchmark puts the hook 3.8x ahead
  (1.66 us -> 0.43 us per copy).

Best of 7 runs, all captures improve because this is on the universal field
path: http.pcap 726.3 -> 605.4 ms (-16.6%), test.pcap 21.0 -> 17.9 ms (-14.7%),
many_interfaces.pcapng 49.4 -> 41.9 ms (-15.2%), profile.pcapng 44.4 -> 38.1 ms
(-14.3%), ipv6.pcap 5.11 -> 4.52 ms (-11.5%). Serialisation of all 14 fixtures
to tree and json, with and without reassembly, stays byte-identical.
FieldBase.length is a property computing struct.calcsize(self.template) afresh on
every read, and both hot loops read it repeatedly for a value that cannot change
between the reads -- FieldBase.unpack read it three times to unpack one field, and
Schema.unpack read it twice more per field. That came to 176908 calcsize() calls
per extraction of examples/captures/http.pcap, for 43110 fields.

- FieldBase.unpack and Schema.unpack now bind it to a local. Nothing mutates
  _length or _template inside any unpack() -- every assignment to either lives in
  __init__, __call__ or pre_process, i.e. on the construction and packing paths --
  so the hoisted read is the same value each use site saw before.

Best of 7 runs: http.pcap 605.4 -> 593.6 ms (-2.0%), profile.pcapng 38.1 -> 36.9 ms
(-3.0%), many_interfaces.pcapng 41.9 -> 40.9 ms (-2.4%). Fixture serialisation
stays byte-identical.
Schema.unpack() sets every parsed field with setattr(), and the __fields__ branch
of Schema.__setattr__ then marked the schema dirty with 'self.__updated__ = True'.
That assignment is itself an attribute store, so it re-entered __setattr__, failed
the __fields__ membership test, and fell through to object.__setattr__ -- 180297 of
the 254043 primitive __setattr__ calls in an extraction of
examples/captures/http.pcap were this round trip and nothing else.

- write the flag straight into __dict__. It is an instance attribute established
  in __new__ and no class in the Schema hierarchy overrides __setattr__, so this
  is the same store the fall-through performed.

Best of 7 runs: http.pcap 593.6 -> 571.9 ms (-3.7%), profile.pcapng 36.9 -> 35.4 ms
(-4.2%), many_interfaces.pcapng 40.9 -> 39.4 ms (-3.6%). Fixture serialisation
stays byte-identical.
Field classes carry abc.ABCMeta, so isinstance() against them dispatches through
a Python-level ABCMeta.__instancecheck__ instead of the C fast path -- 278123 such
calls per extraction of examples/captures/http.pcap, 11.5% of profiled time. The
loop asked the same 'is this an OptionField' question twice per field.

- bind it to a local and reuse it. Nothing between the two uses can change
  type(field), so the second test could only ever have agreed with the first.

Best of 7 runs: http.pcap 571.9 -> 566.3 ms (-1.0%), profile.pcapng 35.4 -> 35.0 ms
(-1.2%), test.pcap 17.0 -> 16.8 ms (-1.2%). Fixture serialisation stays
byte-identical.
…suspects

Working notes from the pre-1.5.0 profiling pass, kept so the numbers and the
dead ends survive the session. Covers the five optimisations on this branch with
their before/after, nine findings measured but deliberately not acted on (with
the reasoning, including three routes to the ABCMeta isinstance cost that were
all rejected on clarity or safety grounds), and seven suspects that were
measured and dismissed -- logging, multidict, ProtoChain, _import_next_layer,
Info/InfoMeta, store=True, and in.pcap as a profiling target.

Meant to be folded into docs/source/pep.rst and deleted; it is a scratch record,
not documentation.
The sha table named the pre-rebase commits, which no longer exist. Also records
that the rebase onto 42eb0d9 brought in only examples/benchmark/** and the
Makefile, leaving pcapkit/ untouched, so the measurements still describe this
branch's code.
…twice"

This reverts commit 1794c43.

Hoisting the isinstance() result into a local costs mypy its type narrowing:
storing the test in `is_option` rather than testing inline leaves `field` as
FieldBase in the branches that follow, so `field.option_padding` no longer
resolves and mypy gains four attr-defined errors it did not have before
(124 -> 128 across the package).

The win was 1.0% on http.pcap. Four new static-analysis errors and a flag
variable in place of a self-evident test is not a trade worth making for that,
so the duplicated test goes back. Recorded in PROFILING-NOTES.md as measured
and rejected rather than left for someone to rediscover.
…tionale

Completes the profiling record with the two shapes that were still outstanding,
which between them turned up the largest opportunities of the whole pass:

- IP reassembly calls analyze() eagerly on every frame, fragmented or not
  (reassembly/ip.py:189), because toolkit/pcap.py:53 filters only on DF. On
  http.pcap that re-parses 1117 of 1117 unfragmented frames and is 86% of the
  feature's cost, plus a 133 ms GC bill.
- The PCAP flow dumper reopens the output file per frame and rebuilds a whole
  Frame to obtain bytes it already holds (dumpkit/pcap.py:94 and :128) -- 80% of
  the flow-tracing cost, with 330/331 output files byte-identical under the
  counterfactual.

Also records that reassembly=True and trace=True are master switches that do no
per-protocol work without ip=/tcp= (a benchmark passing only the switch measures
nothing), that timing several shapes in one process inflates them by up to 73%,
and why change 5 was reverted for costing mypy its type narrowing.
The note cited the pre-amend sha for the revert of change 5.
The branch reports 782 passed / 17 skipped; a tree with only the four touched
source files reverted reports 764 / 35. Same 799 collected, zero failures either
side -- the 18-test gap is tests/test_tier_guard.py skipping because the
comparison tree is an unpacked git archive rather than a checkout, which its skip
reason states outright. Records that so nobody reads the two numbers as a
regression, and notes that comparison suites must run inside a real checkout.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new charset-detection cache can retain large byte strings in memory via a public decode path, and the profiling reproduction notes currently include author-specific absolute paths.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR focuses on reducing packet extraction time in pcapkit by optimizing hot-path field operations and charset detection, while documenting the profiling methodology and results for future reference.

Changes:

  • Memoises chardet.detect() results and reuses the cache across both ProtocolBase.decode() and StringField.post_process().
  • Speeds up field copying and reduces repeated Field.length property computation in unpack loops.
  • Avoids recursive Schema.__setattr__ re-entry during per-field assignment, reducing attribute-set overhead.
File summaries
File Description
PROFILING-NOTES.md Adds detailed profiling measurements, reproduction steps, and ruled-out hypotheses.
pcapkit/protocols/schema/schema.py Avoids __setattr__ self-recursion; hoists field.length reads to locals during unpack.
pcapkit/protocols/protocol.py Routes decode-time charset detection through the new cached helper.
pcapkit/corekit/fields/strings.py Introduces _detect_charset with bounded lru_cache; uses it in string post-processing.
pcapkit/corekit/fields/field.py Adds a fast __copy__ implementation; hoists length in unpack() to avoid repeated calcsize work.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pcapkit/corekit/fields/strings.py Outdated
Comment thread PROFILING-NOTES.md Outdated
JarryShaw added a commit that referenced this pull request Sep 16, 2026
…answered it

"Maybe Even Faster?" closed by asking for "a measured benchmark showing where the
time actually goes". That benchmark now exists (#420) and cut extraction on a
1117-frame HTTP capture by ~46% with byte-identical output, so the section asks
for something already delivered.

Rewritten to record what was found instead: the four hot-path defects that were
fixed, the three larger wins deliberately left for their own review (the flow
dumper reopening its output per frame, `analyze()` running eagerly on
unfragmented frames, and every option being parsed twice), and the two
measurement traps that make a naive benchmark of this library meaningless --
`reassembly=`/`trace=` are no-ops without `ip=`/`tcp=`, and timing several
capture shapes in one process inflates them by up to 73%.

Two predictions the profiling contradicted are recorded as such, since both are
the sort of thing a contributor would otherwise assume and spend a day on:
`Info` construction, the usual suspect, is 0.0% of its own cost and 4.4% of a
parse; and the logging integration is 0.03% even in the heaviest shape. The
IO-batching idea the original thread proposed was never the bottleneck.

Whether unfragmented frames should be emitted as datagrams at all is left as an
open design question rather than answered here.

Clean build: 91 warning/error lines, unchanged from this branch's baseline, none
naming pep.rst.
… notes

Two review findings on #420, both right.

`lru_cache` bounds how many entries it keeps, not how large they are, and
`ProtocolBase.decode` is public -- so a caller handing it whole payloads could
retain 1024 of them for the life of the process. Values above 256 octets now
bypass the cache.

The threshold is measured rather than guessed: across `http.pcap`, `http6.cap`
and `many_interfaces.pcapng`, every value reaching detection was at most 116
octets with a 95th percentile of 52, so 256 keeps every repeating string a real
capture presents. A payload large enough to bypass is unlikely to recur anyway,
so it loses nothing it was not already paying.

The reproduction snippet in the notes hard-coded this machine's absolute paths,
which are meaningless to anyone else; it now derives the tree from
`git rev-parse --show-toplevel`, and says the absolute figures are one machine's
while the ratios are what carry.

Verified: short values still cached (1 miss, 2 hits), long values bypass
(`currsize` unchanged), both paths return what `chardet.detect` returns.
`http.pcap` best-of-7 at 565.5 ms against 560.7 before, so the win holds. Full
suite 782 passed, 17 skipped.
JarryShaw added a commit that referenced this pull request Sep 16, 2026
…ted page (#419)

* docs: record the protocol, reassembly and crypto gaps on the Help Wanted page

Catalogues what PyPCAPKit does not yet implement, so the gaps are tracked
rather than rediscovered. Every entry below was verified against the code at
5e4378d; nothing here is inferred from documentation.

New sections in docs/source/pep.rst
-----------------------------------

* "DTLS" (under More Protocols, More!!!) -- no dissector and no stub anywhere
  in the tree; only TLS/SSL has a stub. The registries have moved ahead of it:
  SCTP DTLS chunk type 65 (pcapkit/const/sctp/chunk.py:77), DTLS Key Management
  chunk parameter 0x8006 (const/sctp/parameter.py:86), error causes 100-103
  (const/sctp/cause_code.py:62,66,70,74), and seven PPIDs -- 47, 66, 67, 68, 69
  and 4242 (const/sctp/payload_protocol_identifier.py:181,240,243,246,249,265).
  DTLS blocks NGAP over DTLS (PPID 66) independently of NGAP itself.

* "Registered, But Not Dissected" (under More Protocols, More!!!) -- registry
  values with no dissector behind them, all counts measured by importing the
  worktree copy:
    - LinkType 3 of 219 bound (ETHERNET 1, IPV4 228, IPV6 229), declared twice:
      pcapkit/protocols/misc/pcap/frame.py:89-96 and misc/pcapng.py:555-562
    - TransType 15 of 151 bound, pcapkit/protocols/internet/internet.py:89-107
    - EtherType 6 of 160 bound, pcapkit/protocols/link/link.py:74-87
    - AppType 2 port numbers of 8182 -- TCP 21, TCP 80 (transport/tcp.py:309-315)
      and UDP 80 (transport/udp.py:72-78)
    - SCTP PPID 0 of 75; SCTP.__proto__ ships empty, transport/sctp.py:344-347,
      and no in-tree caller of register_sctp exists
  Cheap wins named: OSPF (link/ospf.py:70) and L2TP (link/l2tp.py:76) are
  implemented but bound to nothing (TransType 89, 115 unbound; both make
  __index__ raise at ospf.py:229-237 / l2tp.py:231-239); FTP_DATA
  (application/ftp.py:195) exists but TCP 20 is unbound; HTTP is bound on port
  80 only; EtherType 0x88A8 unbound though VLAN parses that shape; LinkType
  NULL/LOOP/RAW carry bare IPv4/IPv6 which pcapkit dissects.

* "Reassembly Beyond IP and TCP" (new top-level section) -- SCTP has no
  reassembly; grep -rni sctp pcapkit/foundation/ returns nothing.
    - The DATA chunk fields a reassembler needs are all exposed already:
      tsn/stream_id/stream_seq/ppid/data and flags.B/flags.E at
      protocols/transport/sctp.py:1143-1157; the only readers of B/E are in
      _make_chunk_data (sctp.py:1725-1726), i.e. the construction path.
    - Only the first DATA chunk of a bundle is dispatched, sctp.py:517-524.
    - I-DATA (RFC 8260, chunk 64) is not dissected -- SCTP.__chunk__[Chunk(64)]
      is 'donone'; read() matches only Payload_Data (sctp.py:520), so an
      I-DATA-only packet yields no payload at all. FORWARD TSN 192 and
      I-FORWARD-TSN 194 likewise unhandled.
    - Where the work goes: pcapkit/foundation/reassembly/sctp.py, subclassing
      Reassembly and implementing reassembly() and submit() (the only two
      abstract methods; reassembly/reassembly.py:180-195). Registering it is
      inert today -- Extractor.register_reassembly accepts any name
      (extraction.py:396-416) but Extractor instantiates only three literal
      keys (extraction.py:840,848,856) and ReassemblyManager is a fixed-field
      Info container (reassembly/__init__.py:37-49). Also needs the read-side
      twin (reassembly/data/__init__.py:37-48), five toolkit adapters, five
      engine call sites, and interface/core.py:148-171.
    - Flow tracing is TCP only (Extractor.__traceflow__ has one key;
      TraceFlowManager one field). TCP tracer closes on FIN not RST -- rst is
      absent from traceflow/data/tcp.py:43-46 -- and flows are unidirectional
      (traceflow/tcp.py:99).
    - No reassembly timeout or eviction anywhere in pcapkit/foundation/; the
      buffer models carry no timestamp (reassembly/data/ip.py:29-51,
      data/tcp.py:29-61). 8191 + 65535 bytes preallocated per in-flight IP
      datagram id (reassembly/ip.py:82-88).
    - Engines: pyshark disables reassembly (engines/pyshark.py:207-213);
      pypcap (pypcap.py:295-306) and pcap_ct (pcap_ct.py:380-391) disable
      reassembly and traceflow; pypcapfile disables IPv6 (pypcapfile.py:225-229,
      toolkit/pypcapfile.py:348-350).

Corrections to text that was already on the page
------------------------------------------------

* ESP. "Only five encryption and five integrity algorithms" is right (verified:
  36 Cipher members, 15 Integrity, 5 entries each in CIPHER_SUITES at
  esp.py:405-426 and INTEGRITY_SUITES at esp.py:432-448) but two of the ten are
  the no-ops ENCR_NULL and NONE. The "not implemented" list was incomplete;
  added AES-CTR (RFC 3686), AES-CMAC-96 (RFC 4494), the AES-GMAC family
  (RFC 4543) with ENCR_NULL_AUTH_AES_GMAC, and the Camellia, RFC 8750
  implicit-IV and RFC 9227 MGM families. Added the two structural blockers:
  IntegritySuite records the digest as a hashlib constructor name
  (esp.py:360-362, 687-693) so a non-HMAC MAC needs the record widened, and
  load_cryptography imports only the ciphers submodules (esp.py:223-234).

* Mobility Header. The three counts are accurate (24 messages / 14 dispatched,
  71 options / 20, 4 CGA extensions / 1; tables at mh.py:527, 552, 583) but the
  claim that the "# TODO markers sit at the exact dispatch tables" was wrong --
  the six markers are at mh.py:1510, 2310, 2422, 2986, 3757, 3871, i.e. the end
  of each handler block. Both places need editing. Also noted that read and make
  are symmetric (37 each) so every gap is a gap in both directions, and that
  mh.py:32-78 imports 32 const/mh sub-registry enums it never uses -- the
  enumerations for the missing options are already generated.
  "NEMO" dropped from the option-group description: Mobile Network Prefix
  (option 6) is implemented, while DNS-UPDATE-TYPE (17), Vendor Specific (19)
  and Service Selection (20) are missing and belong to none of the named groups.

* Fixed one pre-existing rendering bug in the Logging Integration bullet list: a
  literal nested inside bold. RST has no nested inline markup and does not warn,
  so the backticks were rendering as visible characters.

Checked and confirmed still accurate, so deliberately left alone
----------------------------------------------------------------

* The NotImplemented stub list -- 33 files, all 0 bytes except
  link/NotImplemented/eapol.py which holds one comment line. Layer grouping in
  the page matches the tree exactly, including NDP being under link/ and QUIC
  having no stub.
* SCTP registered-but-unimplemented type codes: 30 chunk types / 13 in
  __chunk__, 32 parameters / 8, 23 cause codes / 13 -- so 17, 24 and 10 missing,
  exactly as the page says.
* Test suite: 91 modules matching tests/test_*.py.
* The warnings double-report item: pcapkit/utilities/warnings.py documents the
  two-channel emission as deliberate, and warn() does still log and then call
  warnings.warn, so an application routing warnings into logging sees each twice.
* ESP/SCTP/PCAPNG/logging/engines all genuinely done, as the page states.
* No read/make asymmetry anywhere: NotImplementedError appears only in
  utilities/exceptions.py definitions, and every UnsupportedCall in
  pcapkit/protocols/ is a deliberate design decision (extension headers with no
  payload of their own, container classes with no registry index, abstract
  bases).

Code defects found while auditing, not fixed here -- to be filed as issues
-------------------------------------------------------------------------

1. __proto__ registry pollution. protocol.py:1300 does self.__proto__[proto] on
   a defaultdict, so a lookup miss inserts the key; internet/internet.py:248 and
   internet/ipv6.py:409 repeat it. Reproduced: parsing one 24-byte IPv4 packet
   with proto=1 inserts TransType.ICMP -> Raw into the class-level (shared)
   Internet.__proto__, after which register_transtype(TransType.ICMP, ...) warns
   "protocol 1 already registered, overwriting". SCTP._import_next_layer
   overrides the base method precisely to avoid this and documents the bug at
   sctp.py:818-831 -- a ready-made template for Frame, PCAPNG, Link, Internet,
   IPv6, TCP and UDP.
2. beholder's Raw fallback breaks on payload-less schemas.
   utilities/decorators.py:137 calls self.__header__.get_payload(), which raises
   ProtocolUnbound (schema/schema.py:461) for SCTP, whose schema has no payload
   field. Any registered SCTP upper layer that throws takes the whole SCTP layer
   down to Raw. Already fixed on the open NGAP branch (PR #417), so main only.
3. register_apptype cannot register an SCTP-only service. It raises RegistryError
   (foundation/registry/protocols.py:633-634) for the ~41 AppType members whose
   proto is SCTP-only or DCCP-only, and register_sctp is PPID-keyed, so there is
   no alternative path. Intent unclear.
4. ESP AEAD dispatch falls through. esp.py:743-756 and 782-806 treat anything
   that is not ENCR_NULL or ENCR_AES_CBC as AES-GCM. Safe today because
   CIPHER_SUITES holds nothing else, but adding AES-CCM or ChaCha20-Poly1305 to
   the table without touching decrypt/encrypt would silently mis-process them.
5. Cipher.get() / Integrity.get() invent members for an unknown string via
   extend_enum(..., -1) (const/esp/cipher.py:229-233, integrity.py:118-122).
   ESP's own _resolve avoids them, so not reachable through ESP.
6. vendor/pcapng/filter_type.py:25 has DATA = {} with a TODO URL, so the
   generated enum has no members at all.
7. DPKT and Scapy trace output is affected by the mapping-vs-frame defect that
   extraction.py:879-881 documents and excludes. PR #412 is in flight over that
   area.

Docs build: 91 WARNING/ERROR lines before this change. The after-count is still
running at the time of this commit and is NOT yet confirmed; re-run
`PCAPKIT_SPHINX=1 PYTHONPATH=. python -m sphinx -b html docs/source /tmp/out`
and compare before treating this as verified.

Not done yet: the summary posts to discussion #106 (one reply under the "More
Protocols, More!!!" topic comment 2859991, one new top-level comment for the
reassembly theme, which no existing topic covers) and the DTLS note under #251.
Nothing has been posted to either discussion.

* docs: reconcile the performance section with the profiling pass that answered it

"Maybe Even Faster?" closed by asking for "a measured benchmark showing where the
time actually goes". That benchmark now exists (#420) and cut extraction on a
1117-frame HTTP capture by ~46% with byte-identical output, so the section asks
for something already delivered.

Rewritten to record what was found instead: the four hot-path defects that were
fixed, the three larger wins deliberately left for their own review (the flow
dumper reopening its output per frame, `analyze()` running eagerly on
unfragmented frames, and every option being parsed twice), and the two
measurement traps that make a naive benchmark of this library meaningless --
`reassembly=`/`trace=` are no-ops without `ip=`/`tcp=`, and timing several
capture shapes in one process inflates them by up to 73%.

Two predictions the profiling contradicted are recorded as such, since both are
the sort of thing a contributor would otherwise assume and spend a day on:
`Info` construction, the usual suspect, is 0.0% of its own cost and 4.4% of a
parse; and the logging integration is 0.03% even in the heaviest shape. The
IO-batching idea the original thread proposed was never the bottleneck.

Whether unfragmented frames should be emitted as datagrams at all is left as an
open design question rather than answered here.

Clean build: 91 warning/error lines, unchanged from this branch's baseline, none
naming pep.rst.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are cohesive, performance-motivated, and I did not find any correctness or maintainability issues in the modified hot-path code.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment thread pcapkit/corekit/fields/strings.py Outdated
Comment thread PROFILING-NOTES.md Outdated
Comment thread pcapkit/corekit/fields/strings.py Outdated
Three review findings on #420.

`_detect_charset` lived in `corekit/fields/strings.py` and was imported into
`protocols/protocol.py` as a private cross-module name, which was the wrong shape
for something two callers share. It is now `pcapkit.utilities.chardet`, exported
as `detect_charset` and documented alongside the other utilities.

The module is named for the third-party package whose domain it covers, matching
`utilities/logging.py` and `utilities/warnings.py`, which are named for the
stdlib modules they extend. That does not shadow anything: absolute imports mean
`import chardet` inside it still resolves the real one (verified, 7.6.0). The now
unused `import chardet` in `strings.py` goes too.

`PROFILING-NOTES.md` is dropped rather than converted. `MANIFEST.in` carries
`global-include *.rst`, so renaming it to `.rst` at top level would newly ship
developer notes in every sdist, and `docs/` is pruned from the sdist but would
make it a Sphinx source needing a toctree entry. Its substance is already in
`docs/source/pep.rst`'s performance section, which #419 merged -- the findings,
the wins, the ruled-out suspects and the two measurement traps -- and the exact
call counts and per-capture figures are in #420's description permanently.

The prefix-caching suggestion is answered in the docstring rather than only in
the review thread, since the next reader will meet the bypass and wonder: keying
on the first N octets is unsound because `chardet` is statistical over the whole
sequence, measured at three disagreements in six realistic cases, each of which
would decode a non-ASCII body as ASCII.

Full suite 782 passed, 17 skipped.
Comment thread pcapkit/utilities/chardet.py Outdated
Per review on #420, `detect_charset` is now `pcapkit.utilities.chardet.detect` --
the same convention as `utilities/warnings.py` exporting `warn` for
`warnings.warn`, so the wrapper reads like the thing it wraps.

`import chardet` inside a module named `chardet` still resolves the third-party
package, since imports are absolute; verified at 7.6.0, and `import pcapkit`
is unaffected.

I briefly replaced the `:func:`chardet.detect`` references with plain literals,
on the theory that a module named `chardet` would make them ambiguous -- Sphinx's
Python domain resolves an unqualified target by suffix match, and
`pcapkit.utilities.chardet.detect` ends with `chardet.detect`. The rendered HTML
says otherwise: all three resolve to
`chardet.readthedocs.io/en/latest/api/index.html#chardet.detect` while the local
`detect` resolves to `#pcapkit.utilities.chardet.detect`. Intersphinx wins here,
so the roles are restored and the note explaining the non-problem is gone.

Full suite 830 passed, 17 skipped. Docs build adds no warning naming the page.
…hed too

Per review on #420: the size threshold was the wrong call, and the reason given
is the right one -- it leaned on the sample captures being representative of real
traffic, and they are not. A capture full of large repeated text is precisely the
case that most wants the cache, and it was the one case the threshold excluded.

The cache is now keyed on a 32-octet `blake2b` digest, so what it retains is
bounded by count *and* size -- 1024 x 32 octets, whatever is passed -- while every
value benefits regardless of length. Measured: 1524 distinct 5 KB values leave
1024 entries holding ~32 KiB rather than 5 MiB, and a 2 MB value caches under a
32-byte key.

An `OrderedDict` with `move_to_end`/`popitem` rather than `lru_cache`, because the
key is a digest of the argument rather than the argument, which `lru_cache` cannot
express.

The cost is real and worth stating: `http.pcap` best-of-7 goes 565.5 ms to
588.0 ms, ~4%, from hashing 3011 short strings that the previous version keyed
directly. Against the 1042.7 ms baseline it is still -43.6%. A hybrid keyed on raw
bytes below the threshold and on a digest above would recover that 4% and stay
bounded, at the price of two key paths; not worth it unless the 4% is.

Verified identical to `chardet.detect` on ASCII-prefixed UTF-8, Latin-1 and
UTF-16 inputs. Full suite 830 passed, 17 skipped.
Comment thread pcapkit/utilities/chardet.py Outdated
Your call on #420, and it turns out to be the fastest of the three variants as
well as the simplest: `http.pcap` best-of-7 at 559.8 ms, against 565.5 for the
size-threshold version and 588.0 for the digest-keyed one, from 1042.7 baseline.

`detect` is now just `lru_cache` over `chardet.detect`, and the module is 70 lines
shorter for it. `detect.cache_clear()` comes free with that, which is a real gain
-- a long-running consumer now has a way to release the cache, which the
hand-rolled `OrderedDict` did not offer.

The trade-off is recorded in the docstring rather than left for someone to
rediscover, with the measurements behind it: `lru_cache` caches an argument's
*hash* but still keeps the argument, because a dict needs the key to settle
equality on a hash collision -- 20 distinct 1 MB values retain 19.1 MB, where the
digest-keyed version retained 0.0 MB for the same input. So the bound is on entry
count, not footprint. Both rejected alternatives are named there too, so the next
reader does not re-run this: prefix keys are unsound (chardet is statistical over
the whole sequence, three disagreements in six realistic cases), and digest keys
work but cannot be expressed with `lru_cache`, which keys on what it is passed.

Full suite 830 passed, 17 skipped.
@JarryShaw
JarryShaw merged commit d5f13d4 into main Sep 17, 2026
24 checks passed
@JarryShaw
JarryShaw deleted the perf/hot-path-wins branch September 17, 2026 01:06
JarryShaw added a commit that referenced this pull request Sep 17, 2026
…ayloads lazily (#424)

* reassembly: make the four IPv6 adapters agree, and stop advertising IPv6-Frag (#415)

The four toolkit adapters disagreed about whether the 8-octet IPv6 Fragment
header belongs to a reassembly packet's `ihl`, `header` and `tl`. On
`examples/captures/ipv6.pcap` frame 13, `(ihl, len(header), tl)` read
`(48, 48, 1496)` for `pcap` and `pcapng`, `(40, 40, 1488)` for `dpkt` and
`(40, 40, 1496)` for `scapy` -- no two adapters agreeing on all three, and a
three-way split on `tl` alone. RFC 8200 section 4.5 settles it: the Fragment
header is not present in the reassembled packet, so none of the three may count
it. All four now report `(40, 40, 1488)`.

* `toolkit/pcap.py`, `toolkit/pcapng.py`: `ihl` and `header` were
  `ipv6_info.hdr_len` and the whole of `ipv6_info.fragment.header`, both of
  which count the Fragment header, because `ipv6.py` adds each extension
  header's length before the Fragment-header check breaks its loop. Subtract
  the Fragment header's own length back off, undoing exactly that addition.
  `hdr_len` keeps its documented meaning.
* `toolkit/scapy.py`: `tl` was `len(ipv6)`, which counts the Fragment header
  while its `ihl` did not. The reassembly machinery writes each fragment's
  payload over the span `tl - ihl`, so that overstated it by 8 and the
  reassembled datagram came out 4786 octets instead of 4778 -- eight stray
  zeroes per fragment. Derive `tl` from the payload handed over instead, which
  is what makes the invariant structural rather than a coincidence.
* `toolkit/dpkt.py`: already correct on all three fields; only its `header`
  comment, which called a `bytes` value a `bytearray`, needed fixing. The
  `# header length, only headers before IPv6-Frag` comment repeated in all four
  was already true here and in `scapy`, and is now true in the other two.
* `reassembly/ip.py`, `reassembly/ipv6.py`: excluding the Fragment header's
  octets still left the field *pointing* at it, so every engine reassembled a
  datagram whose `header[6]` was 44. RFC 8200 section 4.5 also moves the
  Fragment header's Next Header value into the last header of the
  unfragmentable part, so `IP._rectify_header` is a hook the IPv6 subclass
  overrides to do that once for all four engines. It walks the extension header
  chain rather than assuming offset 6, since Hop-by-Hop, Routing and
  Destination Options headers may precede the Fragment header.

Deliberately not changed: the reassembled header's Payload Length field still
describes the first fragment. It cannot be computed from one fragment, and
`len(payload)` already gives the datagram's length; the docs now say so.

Tests: `tests/integration/test_reassembly_engine_parity.py` runs all four
adapters over the same octets -- the frames of `ipv6.pcap`, rewrapped as
PCAP-NG for the adapter with no fragmented sample of its own -- and asserts they
agree; on the pristine tree it fails with the table above.
`tests/foundation/reassembly/test_ipv6.py` covers the chain walk, including the
Authentication Header's different length encoding.

`tests/toolkit/test_pcap_unit.py` was pinning the old behaviour: it asserted
`v6.header == b'V' * 48`, i.e. a header with the Fragment header still in it,
for both the PCAP and PCAP-NG adapters. Updated to 40, with `ihl`, `tl` and the
`tl - ihl == len(payload)` invariant pinned alongside; its IPv6 fake gained the
`length` attribute the real `IPv6_Frag` has and its `raw_len` now agrees with
its own payload.

822 passed, 17 skipped. mypy 124 errors either side, pylint message multiset
identical (bar `cyclic-import`, which is non-deterministic run to run: 115 then
124 on two consecutive runs of the unchanged tree).

* reassembly: analyse a reassembled datagram's payload on first read (#420)

`IP.submit` called `Protocol.analyze` eagerly on every datagram it built, and IP
reassembly builds one for *every* frame -- not only the fragmented ones, because
`toolkit/pcap.py` dismisses an IPv4 frame only when its DF flag is set, so a
frame with `DF=0, MF=0, FO=0` reaches `ip.py:73`, allocates a buffer and is
submitted as a trivially complete datagram. `http.pcap` holds 1117 IPv4 frames,
none of them fragmented, and yielded 1117 "datagrams" -- each one a second full
parse of a payload most callers never look at.

`Datagram.packet` is now analysed on first read. `Deferred` holds the bound
analyser, the protocol type and the payload, and `Datagram.__analyse__` runs it
once and keeps the result.

`Info` builds its mapping view out of `__dict__`, so a naively lazy field
disappears from `to_dict()`, `keys()` and `repr()` -- or shows up there as the
placeholder. Listing `packet` in `__additional__` is what avoids that: `Info`
already stores a field whose name collides with a builtin name under a mangled
key and maps it back on the way out, so the key stays in every view under its own
name while attribute reads fall through to `__getattr__`. `__getitem__`,
`__str__`, `__repr__` and `to_dict` resolve before answering; `__contains__` does
not, since `Mapping.__contains__` would otherwise run a full parse just to decide
the field exists.

`bytes(datagram[:TDL])` is now taken once rather than twice, which is why forcing
every `packet` is slightly cheaper than the old eager path rather than equal to
it.

Measured, `http.pcap`, best of 5, one interpreter per shape:

    shape                            before     after    delta
    baseline (no reassembly)        1046.7ms  1031.1ms        -
    reassembly=True ip=True         1881.1ms  1111.2ms   -40.9%
      i.e. the feature's own cost   +834.4ms   +77.3ms   -90.7%
    ... with every packet read             -  1793.8ms
    reassembly=True ip=True, gc off  1801.3ms  1114.7ms
      i.e. the GC bill                +79.8ms     ~0ms

`test.pcap` (34 frames, 21 trivial datagrams): 40.7ms -> 29.7ms, -27.0%; feature
cost +12.8ms -> +1.3ms, -89.8%.

Behaviour-neutral, checked three ways rather than assumed: every observable of a
`Datagram` -- attribute read, `len`, iteration, `to_dict`, `dict()`, `keys`,
`items`, `get`, `in`, `str`, `repr`, `hasattr`, and `copy`/`deepcopy`/`pickle` --
is identical before and after, for a complete and an incomplete datagram and with
`packet` read first and not at all; every reassembly result over all 14 fixtures
in both the `ip` and `tcp` shapes is identical (1509 lines); and serialising all
14 fixtures to `tree` and `json`, with and without reassembly, is byte-identical
to the pre-#415 tree over 60 files and 33 MB.

Not changed, deliberately: whether an unfragmented frame should be emitted as a
datagram at all. That is a design decision about what consumers see, and it is
the owner's to make -- see the report accompanying this branch.
`reassembly/tcp.py:298` has the same eager `analyze`, but it is driven by FIN/RST
and submits 222 times per `http.pcap` pass rather than 1117, so it is left for a
separate change.

827 passed, 17 skipped. mypy 124 errors either side, pylint message multiset
identical (bar `cyclic-import`, non-deterministic run to run).

* reassembly: move ReassemblyData and Deferred into their own data module

Per review on #424. `Deferred` was in `data/ip.py`, which misfiled it: it holds an
analyser, a protocol and a payload, and calls the analyser -- nothing in it is
IP-specific. `ReassemblyData` was in `data/__init__.py`, which is the same
problem from the other side, a class living in a package initialiser.

Both now sit in `pcapkit/foundation/reassembly/data/data.py`, following
`pcapkit/protocols/data/data.py`, and `data/__init__.py` is re-exports only.
`Deferred` remains importable from `data.ip` for anyone who already had it.

Its docstring now separates the mechanism from the case that motivated it: IP
reassembly submits a datagram per frame, which is why the eager parse cost 86% of
IP reassembly over a capture with no fragments, but nothing about postponing the
parse is IP-only. It also records that TCP reassembly builds its `packet` eagerly
too and can use this unmodified -- being FIN/RST-driven, 222 submits per
`http.pcap` pass against 1117, it is a far smaller cost and wants its own change.

Flow tracing was considered and needs nothing: its data models carry no parsed
protocol at all -- `Packet` holds the already-extracted frame it was handed, and
`Buffer`/`Index` hold a dumper, indices and a label -- and there is no `analyze()`
call anywhere in `foundation/traceflow/`. Its per-packet cost was the dumper
rebuilding a `Frame`, a different problem fixed separately.

Full suite 851 passed, 17 skipped.

* traceflow: move TraceFlowData into its own data module, matching reassembly

Symmetry, per review on #424. `TraceFlowData` sat in `traceflow/data/__init__.py`
exactly as `ReassemblyData` sat in the reassembly one; both packages now keep
their shared model in `data/data.py` and their `__init__` as re-exports only,
following `pcapkit/protocols/data/data.py`.

No `Deferred` here, because there is nothing yet to defer: flow tracing holds no
parsed protocol and no payload bytes. `Index` carries a *filename*, a tuple of
frame indices and a label, and `submit()` never sees packet data at all -- frames
go straight to the dumper as they arrive rather than accumulating. Wiring
application-layer analysis into flow tracing is a design change rather than a
refactor, and is being raised separately.

Full suite unchanged.

* reassembly: defer TCP's payload analysis too, sharing the mechanism with IP

Per the decision on #424: flow tracing keeps streaming and gains nothing, while
TCP reassembly -- which already reassembles the stream and already calls
`analyze`, eagerly, in `TCP.submit` -- takes the deferral instead.

The reading half of the arrangement is now a `DeferredPacket` mixin in
`data/data.py` beside `Deferred`, rather than copied into a second `Datagram`.
`IP_Datagram` and `TCP_Datagram` both inherit it and both declare
`__additional__ = ['packet']`, which is what makes the field lazy: `Info` stores a
builtin-named field under a mangled key and maps it back, so `packet` never lands
in `__dict__` and reading it routes through `__getattr__`.

`tcp=True, reassembly=True` over `http.pcap`, best of 5: **1441.0 ms to 1099.0 ms,
-23.7%**. Smaller in relative terms than IP's -90.7%, as expected -- this path is
FIN/RST-driven, 222 submits against 1117 -- but 222 HTTP parses is still 222 parses
most callers never read.

Output is unchanged, checked rather than assumed: 229 datagram lines across
`http.pcap`, `tcp.pcap` and `http6.cap` -- completion, indices, payload lengths and
parsed type -- hash identically before and after. All 222 completed datagrams still
resolve to `HTTP`, memoised on first read, and `'packet' in datagram` still answers
without triggering a parse.

Full suite 851 passed, 17 skipped.

* reassembly: reference the TransType enum instead of hard-coded Next Header values

Per review on #424. The header walk had `44` and `51` as module literals when the
library already names them: they are now `Enum_TransType.IPv6_Frag` and
`Enum_TransType.AH`, so a reader does not have to trust a comment to know which
protocol a number means.

`_NH_IPV6_FRAG` is gone entirely, since the comparison reads better against the
enum member. `_NH_AH` stays as a name -- bound to `Enum_TransType.AH` -- because
its comment carries the reason the walk singles that header out at all: AH is the
one extension header that does not measure its length in 8-octet units
(:rfc:`4302#section-2.2`).

`__protocol_type__` is `None` on `IPv6`, `IPv6_Frag` and `AH`, so there is no
protocol-class attribute to prefer over the const enum here; checked rather than
assumed.

The two remaining literals stay literals, and now say why: `_IPV6_HDR_LEN` (40) and
`_IPV6_NEXT_HEADER` (6) are byte offsets into the header rather than protocol
numbers, so no enumeration carries them.

Full suite 851 passed, 17 skipped; IPv6 and TCP reassembly both still resolve their
deferred payloads (UDP and HTTP).

* reassembly: inline the AH enum, and move the Deferred docs to follow the class

Two review findings on #424.

`_NH_AH` is gone; the comparison reads `Enum_TransType.AH` directly, with the
reason the walk singles that header out moved to the use site rather than living
on a constant that existed only to carry it. Four-adapter parity re-checked after
the change: 40/40/1488 and `header[6] == 17` on default, dpkt and scapy alike.

The docs gap was mine and larger than the comment suggested. `DeferredPacket` was
in `data/data.py`'s `__all__` but never re-exported from the package, so autodoc
could not import it at all; and `Deferred` was documented in
`reassembly/ip/ip.rst` from when it lived in `data/ip.py`, so my new entry beside
`ReassemblyData` made it a duplicate registration. The stale `ip.rst` entry is
removed -- the class moved, so its documentation moves with it -- and the two
`:class:` references in `ipv4.rst` and `ipv6.rst` are repointed. `TraceFlowData`
was already documented; what it lacked was the same treatment for the module it
now lives in.

I also added `.. module::` directives for both `data.data` modules and then
removed them again: they double-register everything the autoclass paths already
cover, which is what produced the duplicate warning.

Clean build: 85 warning/error lines, exactly main's baseline, with all four of
`ReassemblyData`, `Deferred`, `DeferredPacket` and `TraceFlowData` rendering and no
warning naming `data.data`. Full suite 857 passed, 17 skipped.

* docs: name the real module for the shared data classes

Per review on #424: the docs described `ReassemblyData`, `Deferred`,
`DeferredPacket` and `TraceFlowData` by their re-export path, which said where to
import them from rather than where they are. They are now documented as
`...data.data.<class>`, under a `.. module::` directive for each of the two new
modules.

That is also what the sibling pages already do -- `reassembly/tcp.rst` documents
`...data.tcp.Packet` and `ip/ip.rst` documents `...data.ip.Datagram`, both under a
`.. module::` naming the defining module -- so the re-export paths were the odd
ones out, mine included.

The `.. module::` directives are back and no longer duplicate anything: the
earlier duplicate-registration warning came from `Deferred` being documented in
two places at once (the stale `ip/ip.rst` entry, since removed, plus the new one),
not from the directive.

Imports are deliberately unchanged. `extraction.py` still imports from `...data`,
the public re-export, so the documentation reflects where a class lives while
consumers keep a path that does not move when it does.

Clean build: 85 warning/error lines, exactly main's baseline, nothing naming
`data.data`, all four classes rendering under the canonical path and the
`ipv4`/`ipv6` cross-references resolving to it. Foundation tests 163 passed, 11
skipped.
JarryShaw added a commit that referenced this pull request Sep 17, 2026
…to select an option schema

`OptionField.unpack` unpacked the base schema in full to read one field from
it, threw the result away, rewound, and unpacked the option again with the
real schema -- so every option was parsed twice. Measured on
`examples/captures/profile.pcapng`: 2274 schema unpacks for 1137 options.

- Read the base schema's type field alone, exactly the way `Schema.unpack`
  reads one field, so `code` keeps the enumeration type that the `eool`
  comparison and the `OrderedMultiDict` key rely on.
- Rewind by the octets actually read rather than by `len(meta)`, which a
  truncated stream could have over-rewound.
- Pin that each option is unpacked exactly once, that options of differing
  widths decode in sequence, and that the area past the end-of-option-list
  marker still reaches the following padding field.

Safe because the type field is field 0 of every base schema in the package,
always a fixed 1- or 2-octet `EnumField`/`OptionEnumField` with no length
callback -- checked over all 34 `OptionField` declarations and all 18
distinct base schemas. None of those base schemas overrides `pre_unpack`,
and the five that override `post_process` touch only `len`/`length`, never
the type field. The discarded pre-parse's writes into `packet` were not
load-bearing either: the three option schemas that do not re-declare the
base schema's fields (HOPOPT `_SMFDPDOption` and `_QuickStartOption`, IPv4
`_QSOption`) parse identically with and without them, both in isolation and
driven through `OptionField`.

Measured best of 15 (best of 7 for `http.pcap`), one interpreter per
measurement: `profile.pcapng` 43.3 -> 37.6 ms (-13.3%), `test.pcapng`
7.87 -> 7.04 ms (-10.5%), `test.pcap` 28.9 -> 26.3 ms (-8.9%),
`many_interfaces.pcapng` 48.1 -> 44.8 ms (-7.0%), `http.pcap`
1038 -> 980 ms (-5.6%).

Output is byte-identical: all 771 files of the equivalence run -- 30
`tree`/`json` dumps over the 15 fixtures with reassembly and tracing
enabled, 710 flow-trace files, and the recorded warning multisets -- match
exactly, with no change even to the warnings. Suite 836 passed, 17 skipped;
mypy message multiset unchanged at 124 errors.

`len(data)` is deliberately left alone: returning the consumed count from
`Schema.unpack` is the clean fix and that file belongs to #420, a
`file.tell()` delta is not equivalent for the three wrapper schemas whose
`post_process` returns a nested schema, and the provably-equivalent
`__buffer__` sum is worth only ~0.5%.
@JarryShaw JarryShaw added the perf Pull requests that improve performance (perf: subject prefix) label Sep 22, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf Pull requests that improve performance (perf: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants