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.
Describe the bug
Not a correctness bug — a measured hot path. Profiling
extract()onhttp.pcap(1117 frames) attributes 55,846getattrcalls, ~1.7% of the run, tocopy.copyinvoked from the field__call__path. Filed per #575's measurement, where it was the only pcapkit-ownedgetattrcontributor onceaenum.extend_enum's share (93.6%) was excluded.copy.copy(self)is called once per field per packet at 8 sites:pcapkit/corekit/fields/field.py:294,:579pcapkit/corekit/fields/collections.py:84pcapkit/corekit/fields/misc.py:148,:299,:427,:650,:782Reproduction
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 genericcopy.copyroute viaobject.__reduce_ex__/copy._reconstructwas, 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__mutatesnew_selfvia_callbackand_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.