Improve performance of packet dissection and build - #5085
Draft
polybassa wants to merge 1 commit into
Draft
Conversation
Packet.__setattr__ has to resolve field names before it can fall back to
a plain slot assignment, and Packet.__init__ goes through it 22 times for
every layer that is dissected or built. It was the hottest function in
both paths, at 18.8M calls per 21k dissected packets. Initialize the
slots with object.__setattr__ instead. Their types move to class-level
annotations, since that form leaves nowhere to put an inline type
comment.
PacketListField.getfield located the trailing Padding with
"conf.padding_layer in p" followed by "p[conf.padding_layer]": two full
recursive layer traversals per list element, ~2.7us for a two-layer
element. Dissection always appends Padding as the last layer of the
payload chain, so lastlayer() answers the same question in ~0.3us. This
no longer descends into sub-packet fields, where a nested Padding was a
false positive that truncated the list.
copy_fields_dict() and getfield_and_val() went through copy_field_value()
and get_field(), spending two Python frames per field just to reach
self.fieldtype[name]. Index it directly; both helpers remain for
external callers.
Measured against master over 16 packet types, median of 6 interleaved
rounds on a pinned core with a +/-0.6% noise floor:
dissect +7.7%
build (fresh packet) +10.2%
dissect, PacketListField-heavy +13.9% (DNS/DHCP6/SCTP)
rebuild (dissected packet, cached) +2.0%
No API changes. 6600 tests pass, mypy reports no new errors, and flake8
reports two fewer warnings. The 8 remaining test failures need live
OpenLDAP/SMB servers and fail on master too.
AI-Assisted: yes (Claude Opus 5)
Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5085 +/- ##
==========================================
+ Coverage 80.05% 80.56% +0.51%
==========================================
Files 390 390
Lines 96810 96832 +22
==========================================
+ Hits 77499 78015 +516
+ Misses 19311 18817 -494
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Packet.setattr has to resolve field names before it can fall back to a plain slot assignment, and Packet.init goes through it 22 times for every layer that is dissected or built. It was the hottest function in both paths, at 18.8M calls per 21k dissected packets. Initialize the slots with object.setattr instead. Their types move to class-level annotations, since that form leaves nowhere to put an inline type comment.
PacketListField.getfield located the trailing Padding with "conf.padding_layer in p" followed by "p[conf.padding_layer]": two full recursive layer traversals per list element, ~2.7us for a two-layer element. Dissection always appends Padding as the last layer of the payload chain, so lastlayer() answers the same question in ~0.3us. This no longer descends into sub-packet fields, where a nested Padding was a false positive that truncated the list.
copy_fields_dict() and getfield_and_val() went through copy_field_value() and get_field(), spending two Python frames per field just to reach self.fieldtype[name]. Index it directly; both helpers remain for external callers.
Measured against master over 16 packet types, median of 6 interleaved rounds on a pinned core with a +/-0.6% noise floor:
No API changes. 6600 tests pass, mypy reports no new errors, and flake8 reports two fewer warnings. The 8 remaining test failures need live OpenLDAP/SMB servers and fail on master too.
AI-Assisted: yes (Claude Opus 5)