kafkaparser: zero-copy header view, stack-allocate LargeBufferReader#1462
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1462 +/- ##
==========================================
+ Coverage 43.90% 43.92% +0.01%
==========================================
Files 313 313
Lines 34213 34262 +49
==========================================
+ Hits 15022 15050 +28
- Misses 18213 18233 +20
- Partials 978 979 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR reduces heap allocations in the hot TCP parsing path by (1) changing largebuf.LargeBuffer.NewReader() to return a LargeBufferReader by value (enabling stack allocation via escape analysis), and (2) refactoring Kafka header parsing to use a zero-copy KafkaRequestHeader view over *largebuf.LargeBuffer with on-demand scalar accessors and a computed body-offset.
Changes:
- Return
LargeBufferReaderby value fromLargeBuffer.NewReader()and update call sites to pass&rwhere anio.Reader/*LargeBufferReaderis required. - Refactor Kafka parsing to use
KafkaRequestHeaderas a view over*largebuf.LargeBuffer, and update Kafka request/response parsing and tests accordingly. - Add allocation-focused tests (single-chunk and cross-chunk) and reuse readers in the HTTP retry path to avoid extra allocations.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/internal/largebuf/large_buffer.go | NewReader() now returns LargeBufferReader by value; docs updated accordingly. |
| pkg/internal/largebuf/large_buffer_test.go | Updates tests for value-returned readers; adds zero-allocation verification tests. |
| pkg/internal/ebpf/kafkaparser/common.go | Introduces KafkaRequestHeader as a view over *largebuf.LargeBuffer with accessor methods and body-offset computation. |
| pkg/internal/ebpf/kafkaparser/common_test.go | Adjusts header tests to use new header API and largebuf reader. |
| pkg/internal/ebpf/kafkaparser/fetch.go | Updates fetch parsing functions to accept *largebuf.LargeBufferReader and value KafkaRequestHeader. |
| pkg/internal/ebpf/kafkaparser/fetch_test.go | Updates fetch tests to use largebuf readers and new header helpers. |
| pkg/internal/ebpf/kafkaparser/metadata.go | Updates metadata parsing functions to accept *largebuf.LargeBufferReader and use header accessors. |
| pkg/internal/ebpf/kafkaparser/metadata_test.go | Updates metadata tests to use largebuf readers and new header helpers. |
| pkg/internal/ebpf/kafkaparser/produce.go | Updates produce parsing functions to accept *largebuf.LargeBufferReader and value KafkaRequestHeader. |
| pkg/internal/ebpf/kafkaparser/produce_test.go | Updates produce tests to use largebuf readers and new header helpers. |
| pkg/internal/ebpf/kafkaparser/testhelper_test.go | Adds Kafka header constructors for tests (validated and unchecked). |
| pkg/internal/ebpf/kafkaparser/reader_test.go | Removes old bytesReader test helper (replaced by largebuf usage). |
| pkg/ebpf/common/common.go | Changes large buffer cache type to *largebuf.LargeBuffer. |
| pkg/ebpf/common/tcp_large_buffer.go | Uses largebuf.NewLargeBuffer() and returns *largebuf.LargeBuffer from extraction. |
| pkg/ebpf/common/tcp_detect_transform.go | Updates Kafka path to pass *largebuf.LargeBuffer instead of readers; adjusts Postgres reader usage. |
| pkg/ebpf/common/tcp_detect_transform_test.go | Updates tests to construct largebuf.LargeBuffer for protocol detection/parsing. |
| pkg/ebpf/common/kafka_detect_transform.go | Refactors Kafka detection/parsing to accept buffers and construct body readers from header offsets. |
| pkg/ebpf/common/kafka_detect_transform_test.go | Updates Kafka detection tests for flexible-header tagged-fields byte and new APIs. |
| pkg/ebpf/common/http_transform.go | Reuses LargeBufferReader on HTTP retry and passes &r into bufio.NewReader. |
| pkg/ebpf/common/http2grpc_transform.go | Passes &dataReader into http2.NewFramer due to value-returned reader. |
| pkg/ebpf/common/http2grpc_transform_test.go | Updates HTTP2 tests/bench to use largebuf.NewLargeBufferFrom. |
| pkg/ebpf/common/go_kafka_transform.go | Updates Sarama Kafka path to pass *largebuf.LargeBuffer into Kafka processing. |
| pkg/ebpf/common/sql_detect_transform.go | Changes SQL detection buffer type to *largebuf.LargeBuffer. |
| pkg/ebpf/common/sql_detect_transform_test.go | Updates SQL detection tests to use largebuf.NewLargeBufferFrom. |
| pkg/ebpf/common/sql_detect_postgres.go | Uses *largebuf.LargeBufferReader in the iterator and handler signature. |
| pkg/ebpf/common/sql_detect_postgres_test.go | Updates Postgres iterator tests and no-alloc test for &r usage. |
| pkg/ebpf/common/sql_detect_mysql.go | Changes MySQL handler buffer types to *largebuf.LargeBuffer. |
| pkg/ebpf/common/redis_detect_transform.go | Changes Redis detection/status buffer type to *largebuf.LargeBuffer. |
| pkg/ebpf/common/redis_detect_transform_test.go | Updates Redis tests to use largebuf.NewLargeBufferFrom. |
| pkg/ebpf/common/mqtt_detect_transform.go | Changes MQTT detection buffer type to *largebuf.LargeBuffer. |
| pkg/ebpf/common/mqtt_detect_transform_test.go | Updates MQTT tests to use largebuf.NewLargeBufferFrom. |
| pkg/ebpf/common/mongo_detect_transform.go | Changes Mongo parsing helper to accept *largebuf.LargeBuffer. |
| pkg/ebpf/common/fast_cgi_detect_transform.go | Changes FastCGI helpers to accept *largebuf.LargeBuffer. |
| pkg/ebpf/common/fast_cgi_detect_transform_test.go | Updates FastCGI tests to use largebuf.NewLargeBufferFrom. |
| pkg/ebpf/common/couchbase_detect_transform.go | Changes Couchbase parsing to accept *largebuf.LargeBuffer. |
| pkg/ebpf/common/couchbase_detect_transform_test.go | Updates Couchbase tests to use largebuf.NewLargeBufferFrom. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
pkg/internal/largebuf/large_buffer_test.go:1051
- This test ignores ReadN errors during the warm-up cross-chunk read. If a regression causes ReadN to fail here, the test will proceed in a partially-initialized state (or fail later with a less clear panic). Please assert/require that the warm-up ReadN succeeds so failures are reported clearly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Two related changes to reduce allocations in the hot TCP parse path.
KafkaRequestHeaderis now a view over*LargeBufferrather than a copy offixed fields. Fixed-width fields (APIKey, APIVersion, etc.) are read on
demand via scalar accessors; only ClientID is materialised at construction.
NewReader()/NewBodyReader()now returnLargeBufferReaderby value. Atcall sites where the reader doesn't escape (all of the Kafka and HTTP parse
paths), escape analysis can stack-allocate it, eliminating one heap alloc
per packet. Call sites updated to take
&rwhen passing to functionsexpecting
*LargeBufferReaderorio.Reader.Zero-alloc tests added for single-chunk and cross-chunk reads. Scratch
buffer is pre-warmed in the multi-chunk test so repeated resets show 0
allocs. HTTP retry path reuses the same reader
(r.Reset() + rd.Reset(&r))instead of allocating a new one.
Checklist