Skip to content

perf: copy.copy in the field __call__ path costs 55,846 getattr calls (~1.7% of extraction) #730

Description

@JarryShaw

Describe the bug

Not a correctness bug — a measured hot path. Profiling extract() on http.pcap (1117 frames) attributes 55,846 getattr calls, ~1.7% of the run, to copy.copy invoked from the field __call__ path. Filed per #575's measurement, where it was the only pcapkit-owned getattr contributor once aenum.extend_enum's share (93.6%) was excluded.

copy.copy(self) is called once per field per packet at 8 sites:

file:line
pcapkit/corekit/fields/field.py:294, :579
pcapkit/corekit/fields/collections.py:84
pcapkit/corekit/fields/misc.py:148, :299, :427, :650, :782

Reproduction

$ python -c "import cProfile,pstats,io,pcapkit; pr=cProfile.Profile(); pr.enable(); \
  pcapkit.extract(fin='examples/captures/http.pcap', nofile=True, engine='default'); pr.disable(); \
  s=io.StringIO(); pstats.Stats(pr,stream=s).print_callers('builtins.getattr'); print(s.getvalue())"
...
{built-in method builtins.getattr}  <-  55846  0.008  0.008  copy.py:62(copy)

Expected behavior

Fewer copies, or a cheaper one — if the defensive copy can be avoided when the field carries no mutable per-packet state.

Additional context

This path has already been optimised once, so it is not untouched ground: FieldBase.__copy__ (pcapkit/corekit/fields/field.py:306-318) exists precisely because the generic copy.copy route via object.__reduce_ex__/copy._reconstruct was, in its own docstring's words, "one of the costlier things an extraction did". Any further change should start by reading that rationale.

Risk to weigh before changing anything: the copy is defensive. __call__ mutates new_self via _callback and _length_callback, so sharing an instance across frames could alias per-packet state. Establish that a field's post-callback state is genuinely per-packet-invariant before skipping the copy — a wrong answer here is a silent data-corruption bug, not a slow one.

~1.7% is small. Worth doing only if it is provably safe.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementperfPull requests that improve performance (perf: subject prefix)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions