perf: Optimize Protocol extraction by caching dynamically imported modules in ProtocolBase

[Ts-Boom]

This PR resolves a significant performance bottleneck identified during profiling of pcapkit.extract() by eliminating redundant dynamic imports (importlib.import_module and in-function import overheads) executed over thousands of packets.

🔍 The Problem

Profiling the extraction script revealed that ProtocolBase._import_next_layer() and its associated lookups accounted for approximately ~40% of the cumulative total execution time. The underlying issue originates in the fallback mechanism and on-the-fly imports:

1. When encountering unrecognized packet structures, ProtocolBase._lookup_registry() falls back to a default ModuleDescriptor that maps to the Raw protocol. Because this descriptor isn't inherently cached into the main registry on a miss, ModuleDescriptor.klass (which uses importlib.import_module under the hood) was re-evaluating the dynamic import for every single unrecognized packet frame.
2. Furthermore, null-length payloads and manual term signals within _import_next_layer() repeatedly triggered slow in-function import statements per extraction.

💡 The Solution

Introduced ProtocolBase._MODULE_CACHE—a static, class-level dictionary—to guarantee that each unique protocol descriptor is evaluated and dynamically imported exactly once.

• In _lookup_next_layer(), if a returned ModuleDescriptor is requested, the resulting .klass import is securely cached and returned for all future packet checks matching that module and name pair.
• In _import_next_layer(), fallback assignments like NoPayload and Raw classes check the cache dictionary before invoking local imports.

🚀 Impact

This minor caching abstraction drastically slashes the overhead associated with redundant standard module locks and repeated importlib resolutions on large packet captures, improving parse timings uniformly across parsing flows.

[JarryShaw]

Thanks for following up on the profiling suggestion from #551 — this is exactly the right place to have looked, and the diff is small and readable. Two findings, though, and the first is a blocker.

The cache can serve a stale class

_MODULE_CACHE is keyed on a bare f'{module}.{name}' string and is never invalidated for the life of the process. Reproduced on 683cebc44:

cache warmed with: <class 'pcapkit.protocols.misc.raw.Raw'>
after reload, live class is a new object: True
cache serves OldRaw : True
cache serves NewRaw : False

An instance built from the cache-served class then fails isinstance against the live one. The exposure is the two hardcoded keys in _import_next_layer's fast paths and the ~90 class-body ModuleDescriptor(...) registrations resolved through _lookup_next_layer; ProtocolBase.register() is not affected, since it resolves .klass fresh and stores the class rather than the descriptor.

This matters here more than it might elsewhere: retained class-level dispatch state was fixed twice in this release already (#425/#428 at the protocol layer, #560 at the schema layer), and _lookup_registry in this same file deliberately avoids caching a miss, with a comment saying why.

The ~40% looks like a cumulative-time artifact

cProfile on main (8cfd6ab01), http.pcap, 1117 frames, 2.09 s total:

function	calls	tottime	cumtime
_import_next_layer	3351	0.0121 s (0.58%)	1.888 s (90.2%)
_lookup_next_layer	4695	0.0028 s (0.14%)	0.005 s (0.2%)
_lookup_registry	8490	0.0019 s (0.09%)	0.002 s (0.1%)

_import_next_layer is a recursive-descent dispatcher, so it sits in the call chain of every nested layer and cumulative time attributes the whole parse beneath it to that one frame. Its own cost is under 1%, and the lookup chain's combined self-time is too. The actual self-time leaders are aenum.extend_enum, schema/field unpack, and getattr (976k calls) — nothing import-related.

What does hold

Part 1 of your mechanism is accurate: on a registry miss the default ModuleDescriptor is not written back, so Raw does re-resolve per frame. Measured call counts to ModuleDescriptor.klass: 4 on http.pcap (1117 frames — main already write-back-caches on a hit), but 48 of 52 on many_interfaces.pcapng (64 frames). And unlike #551, the saving is directionally real — klass is ~463 ns against ~39 ns for a warm dict hit. It is just that at 4–48 avoided calls per capture the absolute saving is tens of microseconds against multi-hundred-millisecond extractions.

Where that leaves it

I'd rather not take the staleness risk for a saving that small, so I'm inclined to decline this one — but the miss-path observation is a genuine find and worth keeping. If you'd like to pursue it, memoising the default descriptor into the registry on a miss the way a hit is already written back would capture the same benefit without a second never-invalidated cache, and aenum.extend_enum is where the real time goes if you want a larger target.

A full wall-clock A/B is still running; I'll add it if it changes anything, though given the magnitudes I expect it to be inside the noise.

[JarryShaw]

Follow-up on the wall-clock A/B I said was still running. It doesn't change the conclusion, but two results are worth adding, and one is in your favour.

On the path you targeted, the fix does what you said it does. Measured in isolation on the uncached default-factory branch, per-call cost drops by roughly 38–40%, reproducibly across repeated runs. And the steady-state registered-hit path is unaffected either way — your cache check sits behind main's existing registry write-back, so it never executes on the second-and-later lookup of a registered code. So this is not a repeat of #551: there is real cost here and your patch removes it.

At whole-extract() level the difference is not measurable, and I won't quote a number for it. On http.pcap the mechanism predicts roughly 1.6 µs of real saving (4 avoided calls); on the Raw-heavy many_interfaces.pcapng, roughly 16 µs against a ~38 ms extraction. Run-to-run variance on this host is single-digit milliseconds — two to three orders of magnitude larger — and repeated pairs flipped sign between runs. No realistic repetition count resolves that, so any pipeline-level percentage would be indefensible in either direction.

One thing I should have mentioned in my first comment: the diff adds no tests, so nothing guards the cache against the staleness above.

That leaves it where it was — the stale-dispatch risk is the deciding factor, not the performance. If you want to take this further, the productive version is profiling by self-time rather than cumulative time, which is what makes aenum.extend_enum and the schema/field unpack path visible as the actual hot spots. Genuinely appreciate the care in the write-up; the miss-path observation stands on its own even though I'm not taking this patch.

[JarryShaw]

Following up now that our own version has landed: your proposal shipped, with credit.

ec28418c4 (#586, closing #574) implements the optimisation this PR was after, and you are credited by name in both the commit message and the PR body — "Proposed by @Ts-Boom in #563." The profiling that found the cost was yours.

Two things worth telling you about how it ended up, because both are departures from what this PR did.

It caches nothing. Rather than memoising the resolved class, ModuleDescriptor.klass now reads sys.modules first and enters importlib.import_module only when the module is genuinely absent, or when a loaded module lacks the attribute because its body is still executing. The reason is a measurement: importlib.reload rebinds the class inside the same module object, so a memo validated against module identity would not notice that its cached class is stale. Verified directly —

same module object after reload: True
same class object after reload:  False

— which is why the requirement "no stale class survives a reload" and "memoise the class" cannot both be satisfied. sys.modules is the only module cache whose invalidation the interpreter itself maintains.

The wall clock shows nothing, and we are not claiming otherwise. The call-count and per-call wins are real: import_module calls during extract() on many_interfaces.pcapng went 48 → 0, ModuleDescriptor.klass 435.8 → 117.4 ns, and a _lookup_next_layer miss 883.4 → 526.0 ns. But end to end, 15 runs each, the median moved 37.09 → 37.39 ms — the wrong way, by less than one standard deviation, with the predicted 17 µs saving some 25x below the noise floor.

That last point connects to something your PR prompted us to check properly, now #575: profiled by self time rather than cumulative, aenum.extend_enum is 16.7% of a cold extraction while _import_next_layer is 0.58%. The dispatch path was not where the time went. One correction to an assumption in this PR's framing, measured: a function-level from X import Y costs ~162 ns, far less than assumed — import_module at 436 ns was the expensive one.

So this PR can be closed as superseded rather than rejected. Thank you for it — the direction was right and the credit is in the history.