network: dynamic vote compression #6351
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6351 +/- ##
==========================================
- Coverage 51.82% 50.65% -1.17%
==========================================
Files 652 656 +4
Lines 87425 110899 +23474
==========================================
+ Hits 45307 56177 +10870
- Misses 39249 51858 +12609
+ Partials 2869 2864 -5 ☔ View full report in Codecov by Sentry. |
| // not found: send literal and add to window | ||
| hdr1 |= 0 << hdr1PropShift // set 000 | ||
| e.proposalWindow.insertNew(prop) | ||
| // write proposal bytes as StatelessEncoder would |
There was a problem hiding this comment.
consider delegating to stateless encoder/decoder there to have stateful less dependent on stateless impl details?
There was a problem hiding this comment.
I'm not sure how to delegate it though without some interface/function abstraction that will slow it down, because StatelessEncoder's job is to read msgpack and output the compressed format that is an input here, and StatelessDecoder's job is to output msgpack format, so there is no subroutine I can think of that reads a compressed format and does hdr0 & bitDig check (set by StatelessEncoder) and then re-outputs the same compressed format..
| // to 7 entries, encoded using 3 bits in the header byte. This is enough to | ||
| // provide effective compression, since usually almost all the votes in a round | ||
| // are for the same proposal value. | ||
| type propWindow struct { |
There was a problem hiding this comment.
Pull Request Overview
This PR adds dynamic vote compression by introducing StatefulEncoder and StatefulDecoder, leveraging HPACK-style references to prior vote fields to reduce message size.
- Replaced manual varuint byte-length logic in StatelessDecoder with a shared
msgpVaruintRemaininghelper. - Implemented a small sliding window (
propWindow) for proposal entries and a 2-way set-associative LRU table (lruTable) for other vote fields. - Added comprehensive unit tests for proposal window, LRU table, and the new stateful compression layer.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| network/vpack/vpack.go | Updated headerSize comment; refactored varuint decoding in StatelessDecoder |
| network/vpack/msgp.go | Added msgpVaruintRemaining; updated readUintBytes to use it |
| network/vpack/proposal_window.go | New propWindow implementation for HPACK-style proposal compression |
| network/vpack/proposal_window_test.go | Unit tests for proposal window behavior |
| network/vpack/lru_table.go | New 2-way set-associative lruTable for dynamic references |
| network/vpack/lru_table_test.go | Unit tests for LRU table insert, lookup, and eviction |
| network/vpack/dynamic_vpack.go | New StatefulEncoder/StatefulDecoder and shared dynamicTableState |
| network/vpack/dynamic_vpack_test.go | Tests covering stateful compression/decompression and error paths |
Comments suppressed due to low confidence (2)
network/vpack/vpack.go:42
- [nitpick] The comment for headerSize is unclear—consider clarifying that it defines two header bytes (mask and flags) rather than referencing specific encoder types.
headerSize = 2 // 1 byte for StatelessEncoder, 1 byte for StatefulEncoder
network/vpack/proposal_window.go:48
- The loop uses 'for i := range w.size', but w.size is an int, not a slice or array. This will not compile. Change it to a classic for-loop: 'for i := 0; i < w.size; i++ {'.
for i := range w.size {
Summary
This adds to #6276 a StatefulEncoder and StatefulDecoder, that compress votes using HPACK-style references to values from previous votes. It provides better than 50% compression given different table sizes.
Test Plan