perf(corekit): resolve a ModuleDescriptor through sys.modules, not import_module (#574)

CHANGELOG.md

@@ -43,6 +43,7 @@ The largest release since 1.0, and the first recorded here as it happened rather
 - **Fixed** -- which exception a malformed TCP SACK option raised depended on unrelated process state: a clean interpreter raised `ProtocolError` as documented, but a process that had already popped `pcapkit.corekit.fields.misc` from `sys.modules` -- which the `#439` ABC-cache regression tests do in every case's `setUp`/`tearDown` -- raised `FieldValueError` instead, from a different layer entirely, before the documented check was even reached (#525). The cause was `ListField.unpack` resolving `SchemaField` through a function-local import re-run on every call; a module popped and reimported mid-process comes back as a second, distinct class, so `isinstance` against it silently misclassified the field and billed each item by its declared length instead of by what it actually consumed. **Any caller relying on the previously-observed** `FieldValueError` **for this case now gets** `ProtocolError` **instead, deterministically**, matching the method's own docstring. Fixed by importing at module level instead.
 - **Fixed** -- two dropped-keyword/wrong-cast defects flagged in review during this release and never filed until now: HIP's `_make_param_encrypted` passed `cipher=` to a schema with no such field, so the value was silently dropped and an AES-cipher `ENCRYPTED` parameter built through `make` packed without its IV; and IPv6-Route's `RPL.post_process`, which runs on every `Schema.pack` and not only after a parse, assumed `self.addresses` was still the concatenated `bytes` a parse leaves it as, and raised slicing the `list[bytes]` a `make`-built multi-address header actually holds there (#556).
 - **Fixed** -- two more defects #541 exposed rather than caused, both since it let construction reach code that had never run before. `MPTCP.subtype` was still `typing.TYPE_CHECKING`-only, an annotation rather than a field, so `TCP(options=[(Enum_Option.Multipath_TCP, ...)])` raised `AttributeError: ... has no attribute 'subtype'` for every subtype but `MP_JOIN`: the convenience constructor builds a schema in memory and reads it straight back through `_read_mptcp_*` with no byte round trip, so `_MPTCP.post_process` -- the only code that ever set `subtype` -- never ran. Fixed on the construction path (`TCP._make_mode_mp`) rather than by adding a third real field the way `kind`/`length` got in #541: unlike those two, `subtype` is already packed as 4 bits of each subtype's own `test` bitfield, and a second, independent field for the same bits would either double-encode them or need a "derive, don't pack" field kind this library does not have (#566). Separately, `_make_mptcp_capable` wrote `length=20 if rkey is None else 32` where [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.1 gives 12 and 20, and `MPTCPCapable.rkey`'s own condition (`pkt['length'] != 32`) dropped the receiver's key for exactly the length the maker used to mean "key present" -- so a spec-correct, key-absent MP_CAPABLE could not be built at all, and a key-present one silently lost its key on the wire. Both, and the matching guard in `_read_mptcp_capable`, now agree on 12/20. **This changes MP_CAPABLE's packed output**: a 20-octet, key-present option built or parsed under the old code becomes 12 octets with no key, or 20 octets with the key actually present, depending on which the caller meant (#567).
+- **Changed** -- `ModuleDescriptor.klass` reads an already-imported module out of `sys.modules` rather than re-entering `importlib.import_module`, which matters because next layer dispatch resolves a descriptor there on a per-frame path. A registry *hit* holding a `ModuleDescriptor` is resolved once and written back, but a *miss* deliberately is not -- recording a miss in a class-level `collections.defaultdict` is the defect #425/#428 fixed at this layer and #560 fixed at the schema layer -- so every unrecognised frame resolved the same fallback descriptor again: 48 of the 52 `ModuleDescriptor.klass` resolutions an extraction of `many_interfaces.pcapng` performs, and 4 of the 7 on `ipv4.pcap`. `import_module` keeps real per-call work for a module `sys.modules` already holds, so that resolution now costs ~117 ns rather than ~436 ns and the whole miss path ~526 ns rather than ~883 ns, on CPython 3.14.7. **The scale is worth stating plainly: this is not measurable in** `extract()` **wall clock.** 48 avoided calls is ~17 us against a ~37 ms extraction, two orders of magnitude inside this host's run-to-run variance, and the "~40% of cumulative time" reading that prompted the work was an artifact of `_import_next_layer` being a recursive-descent dispatcher -- its *self* time is 0.58%, while `aenum.extend_enum` is 16.7%. What the change is taken for is its shape rather than its speed: nothing memoises the resolved class, anywhere, so `sys.modules` stays the only module cache in play and its invalidation is the interpreter's. A memo of the class would serve the pre-reload class after an `importlib.reload` forever, and an instance of it fails `isinstance` against the live one. Proposed by `@Ts-Boom` in #563, whose profiling found the miss path; the implementation differs because that one added a second, never-invalidated cache of resolved classes (#574).
 
 Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) and `1.5.0b3` (2026-09-19), all published as prereleases and so resolved only by `pip install --pre`. `1.5.0b1` half-shipped: the tag, the GitHub release and the Conda deployments landed, but PyPI rejected the wheel because `twine check` found a Sphinx-only `:mod:` role in `README.rst`, which `pyproject.toml` declares as the dynamic long description. `1.5.0b2` is what reshipped it -- the release workflow is version-driven, so an existing version cannot republish -- and `1.5.0b3` followed the CI change that stops a TestPyPI outage from costing a release its wheels (#497, #498).
 

docs/source/changelog/1.5.0.rst

@@ -420,6 +420,31 @@ pull requests between #326 and #509.
   packed output**: a 20-octet, key-present option built or parsed under the old
   code becomes 12 octets with no key, or 20 octets with the key actually present,
   depending on which the caller meant (#567).
+* **Changed** -- ``ModuleDescriptor.klass`` reads an already-imported module out of
+  ``sys.modules`` rather than re-entering ``importlib.import_module``, which
+  matters because next layer dispatch resolves a descriptor there on a per-frame
+  path. A registry *hit* holding a ``ModuleDescriptor`` is resolved once and
+  written back, but a *miss* deliberately is not -- recording a miss in a
+  class-level ``collections.defaultdict`` is the defect #425/#428 fixed at this
+  layer and #560 fixed at the schema layer -- so every unrecognised frame resolved
+  the same fallback descriptor again: 48 of the 52 ``ModuleDescriptor.klass``
+  resolutions an extraction of ``many_interfaces.pcapng`` performs, and 4 of the 7
+  on ``ipv4.pcap``. ``import_module`` keeps real per-call work for a module
+  ``sys.modules`` already holds, so that resolution now costs ~117 ns rather than
+  ~436 ns and the whole miss path ~526 ns rather than ~883 ns, on CPython 3.14.7.
+  **The scale is worth stating plainly: this is not measurable in** ``extract()``
+  **wall clock.** 48 avoided calls is ~17 us against a ~37 ms extraction, two
+  orders of magnitude inside this host's run-to-run variance, and the "~40% of
+  cumulative time" reading that prompted the work was an artifact of
+  ``_import_next_layer`` being a recursive-descent dispatcher -- its *self* time is
+  0.58%, while ``aenum.extend_enum`` is 16.7%. What the change is taken for is its
+  shape rather than its speed: nothing memoises the resolved class, anywhere, so
+  ``sys.modules`` stays the only module cache in play and its invalidation is the
+  interpreter's. A memo of the class would serve the pre-reload class after an
+  ``importlib.reload`` forever, and an instance of it fails ``isinstance`` against
+  the live one. Proposed by ``@Ts-Boom`` in #563, whose profiling found the miss
+  path; the implementation differs because that one added a second,
+  never-invalidated cache of resolved classes (#574).
 
 Preceded by ``1.5.0a1`` (2026-09-15), ``1.5.0b1`` and ``1.5.0b2`` (both
 2026-09-18) and ``1.5.0b3`` (2026-09-19), all published as prereleases and so

pcapkit/corekit/module.py

@@ -11,6 +11,7 @@
 """
 import collections
 import importlib
+import sys
 from typing import TYPE_CHECKING, Generic, TypeVar
 
 __all__ = ['ModuleDescriptor']
@@ -34,6 +35,46 @@ class can be imported by ``from module import name``."""
 
     @property
     def klass(self) -> 'Type[_T]':
-        """Import class from module."""
-        module = importlib.import_module(self.module)
-        return getattr(module, self.name)
+        """Import class from module.
+
+        Important:
+            The module is read from :data:`sys.modules` first, and
+            :func:`importlib.import_module` is entered only when it is not
+            loaded yet. That matters because this property is on a *per-frame*
+            dispatch path: a next layer code nobody registered falls back to a
+            :class:`ModuleDescriptor` for
+            :class:`~pcapkit.protocols.misc.raw.Raw` which
+            :meth:`ProtocolBase._lookup_next_layer
+            <pcapkit.protocols.protocol.ProtocolBase._lookup_next_layer>`
+            deliberately does not write back, so every unrecognised frame
+            resolves the same descriptor again -- 48 of the 52 resolutions an
+            extraction of :file:`many_interfaces.pcapng` performs.
+            :func:`~importlib.import_module` keeps real per-call work for an
+            already-imported module (locks, :class:`~importlib.machinery.ModuleSpec`
+            checks, the ``fromlist`` walk), so each repeat cost ~436 ns where
+            this property now costs ~117 ns.
+
+            The class is still read off the module with :func:`getattr` on
+            every access, and nothing is memoised here. That is the point:
+            :data:`sys.modules` *is* the module cache, and it is the only one
+            whose invalidation the interpreter maintains --
+            :func:`importlib.reload` rebinds the class in place and
+            ``sys.modules.pop()`` drops the entry, both of which this sees
+            immediately. Holding the resolved class instead would serve the
+            pre-reload class forever, and an instance of it fails
+            :func:`isinstance` against the live one.
+
+        """
+        module = sys.modules.get(self.module)
+        if module is not None:
+            try:
+                return getattr(module, self.name)
+            except AttributeError:
+                # ``sys.modules`` also holds modules whose body is still
+                # executing -- a circular import, or another thread part way
+                # through importing this one. ``import_module`` waits on the
+                # per-module import lock, so defer to it rather than reporting
+                # the attribute missing; a name that really is absent raises
+                # from the ``getattr`` below instead, with the same message.
+                pass
+        return getattr(importlib.import_module(self.module), self.name)

pcapkit/protocols/protocol.py

@@ -1370,6 +1370,16 @@ def _lookup_next_layer(registry: 'DefaultDict[int, ModuleDescriptor[ProtocolBase
             :meth:`self._lookup_registry <ProtocolBase._lookup_registry>` exists
             to avoid.
 
+            So a miss resolves its fallback descriptor again on every frame, and
+            what keeps that affordable is :attr:`ModuleDescriptor.klass
+            <pcapkit.corekit.module.ModuleDescriptor.klass>` reading
+            :data:`sys.modules` instead of re-entering
+            :func:`importlib.import_module` -- see #574. Memoising the resolved
+            class here instead, whether under ``proto``, in ``registry``'s
+            default factory, or in a cache beside the registry, would retain a
+            class that :func:`importlib.reload` then makes stale; #425 and #428
+            at this layer and #560 at the schema layer are all that same defect.
+
         """
         protocol = ProtocolBase._lookup_registry(registry, proto)
         if isinstance(protocol, ModuleDescriptor):

tests/corekit/test_module.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import sys
 import types
 import unittest
 from unittest import mock
@@ -12,6 +13,19 @@ def setUp(self) -> None:
         purge_modules(['pcapkit'])
         self.module = load_module('pcapkit.corekit.module', 'pcapkit/corekit/module.py')
 
+    def _register(self, name: str, module: types.ModuleType) -> None:
+        """Put ``module`` in :data:`sys.modules` for the duration of a test."""
+        original = sys.modules.get(name)
+        sys.modules[name] = module
+
+        def restore() -> None:
+            if original is None:
+                sys.modules.pop(name, None)
+            else:
+                sys.modules[name] = original
+
+        self.addCleanup(restore)
+
     def test_klass_imports_target_attribute(self) -> None:
         target_module = types.SimpleNamespace(Target=dict)
         with mock.patch('importlib.import_module', return_value=target_module) as importer:
@@ -20,6 +34,107 @@ def test_klass_imports_target_attribute(self) -> None:
 
         importer.assert_called_once_with('demo.module')
 
+    def test_klass_reads_an_already_loaded_module_out_of_sys_modules(self) -> None:
+        """An already-imported module must not go through the import machinery.
+
+        :attr:`~pcapkit.corekit.module.ModuleDescriptor.klass` sits on a
+        per-frame dispatch path -- see GitHub issue #574 -- and
+        :func:`importlib.import_module` retains real per-call work even when
+        :data:`sys.modules` already holds the module, so re-entering it for
+        every unrecognised frame costs ~436 ns against the ~117 ns this
+        property takes reading :data:`sys.modules` directly.
+
+        """
+        target_module = types.ModuleType('demo.loaded')
+        target_module.Target = dict  # type: ignore[attr-defined]
+        self._register('demo.loaded', target_module)
+
+        descriptor = self.module.ModuleDescriptor('demo.loaded', 'Target')
+        with mock.patch('importlib.import_module',
+                        side_effect=RuntimeError('import machinery re-entered')) as importer:
+            for _ in range(3):
+                self.assertIs(descriptor.klass, dict)
+
+        importer.assert_not_called()
+
+    def test_klass_follows_a_rebound_class_rather_than_keeping_the_first_one(self) -> None:
+        """Nothing may be memoised, because a reload rebinds the class.
+
+        The two reload idioms differ and both have to be followed.
+        :func:`importlib.reload` re-executes the module body into the *same*
+        module object, so the class it defines is a new object while
+        ``sys.modules[name]`` is unchanged -- which is why a memo validated
+        against the module object's identity would still serve the old class.
+        Popping :data:`sys.modules` and importing again replaces the module
+        object as well. Both are modelled here directly; GitHub pull request
+        #563's class-level cache followed neither, and instances built from the
+        class it kept fail :func:`isinstance` against the live one.
+
+        """
+        class Old:
+            pass
+
+        class New:
+            pass
+
+        target_module = types.ModuleType('demo.reloadable')
+        target_module.Target = Old  # type: ignore[attr-defined]
+        self._register('demo.reloadable', target_module)
+
+        descriptor = self.module.ModuleDescriptor('demo.reloadable', 'Target')
+        self.assertIs(descriptor.klass, Old)
+
+        # what importlib.reload does: same module object, new class object
+        target_module.Target = New  # type: ignore[attr-defined]
+        self.assertIs(descriptor.klass, New)
+
+        # what popping sys.modules and importing again does: new module object
+        replacement = types.ModuleType('demo.reloadable')
+        replacement.Target = Old  # type: ignore[attr-defined]
+        sys.modules['demo.reloadable'] = replacement
+        self.assertIs(descriptor.klass, Old)
+
+    def test_klass_defers_to_import_module_for_a_partially_initialised_module(self) -> None:
+        """A module whose body is still executing is not a resolution failure.
+
+        :data:`sys.modules` holds a module from the moment its body *starts*
+        executing, so a circular import -- or another thread part way through
+        importing the same module -- can see it without the class defined yet.
+        :func:`importlib.import_module` waits on the per-module import lock,
+        which is the behaviour the fast path has to fall back to rather than
+        reporting the attribute missing.
+
+        """
+        partial = types.ModuleType('demo.partial')  # body still running: no Target
+        self._register('demo.partial', partial)
+
+        complete = types.ModuleType('demo.partial')
+        complete.Target = dict  # type: ignore[attr-defined]
+
+        descriptor = self.module.ModuleDescriptor('demo.partial', 'Target')
+        with mock.patch('importlib.import_module', return_value=complete) as importer:
+            self.assertIs(descriptor.klass, dict)
+
+        importer.assert_called_once_with('demo.partial')
+
+    def test_klass_still_raises_attributeerror_for_a_name_that_is_not_there(self) -> None:
+        """A genuinely absent name must still fail, and say so.
+
+        The fallback above swallows one :exc:`AttributeError` to retry through
+        :func:`importlib.import_module`. A descriptor naming a class that does
+        not exist has to come back out of that retry as the same
+        :exc:`AttributeError` it always raised, rather than as :data:`None` or
+        as a second, more confusing error.
+
+        """
+        target_module = types.ModuleType('demo.incomplete')
+        self._register('demo.incomplete', target_module)
+
+        descriptor = self.module.ModuleDescriptor('demo.incomplete', 'Missing')
+        with mock.patch('importlib.import_module', return_value=target_module):
+            with self.assertRaisesRegex(AttributeError, 'Missing'):
+                descriptor.klass  # pylint: disable=pointless-statement
+
 
 if __name__ == '__main__':
     unittest.main()