perf(felt): add CBOR fast path for encode and decode#3792
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3792 +/- ##
==========================================
+ Coverage 75.17% 75.30% +0.12%
==========================================
Files 438 439 +1
Lines 39376 39397 +21
==========================================
+ Hits 29602 29669 +67
+ Misses 7699 7653 -46
Partials 2075 2075 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
08ce659 to
5c42e80
Compare
5c42e80 to
edbd0b1
Compare
|
Claude finished @RafaelGranza's task in 2m 38s —— View job Review: CBOR fast path for
|
450370d to
4897dbf
Compare
|
Claude finished @RafaelGranza's task in 1m 29s —— View job Review: CBOR fast path for
|
4897dbf to
15c2951
Compare
|
Claude finished @RafaelGranza's task in 1m 48s —— View job Review: CBOR fast path for
|
15c2951 to
c6e6b89
Compare
|
Claude finished @thiagodeev's task in 1m 42s —— View job Review: CBOR fast path for
|
thiagodeev
left a comment
There was a problem hiding this comment.
Nice work!
Partial review with some probable improvements
c6e6b89 to
44431a0
Compare
|
Claude finished @RafaelGranza's task in 1m 38s —— View job Review: CBOR fast path for
|
thiagodeev
left a comment
There was a problem hiding this comment.
LGTM.
Only a few comments with nitpicks
1c95608 to
c5fa0a7
Compare
|
Claude finished @RafaelGranza's task in 2m 18s —— View job Review: CBOR fast path for
|
|
Claude finished @RafaelGranza's task in 1m 41s —— View job Review: CBOR fast path for
|
|
@RafaelGranza, initially the PR only added Felt decoding because it showed a clear improvement on the benchmarks. Can you please add benchmarks showing Felt encoding benefits? |
|
Claude finished @RafaelGranza's task in 2m 53s —— View job Task: add encode benchmarks showing Felt CBOR fast-path benefit
@rodrodros — the existing The trick (thanks to your catch on Added to // BenchmarkMarshalCBORGeneric is the pre-fast-path baseline: it routes through
// the reflection-based generic codec by encoding the underlying *fp.Element
// (Felt would dispatch to the fast MarshalCBOR). Compare against
// BenchmarkMarshalCBOR to see the encode speedup.
func BenchmarkMarshalCBORGeneric(b *testing.B) {
for _, tc := range benchCBORInputs {
value := felt.UnsafeFromString[felt.Felt](tc.hex)
element := (*fp.Element)(&value)
b.Run(tc.name, func(b *testing.B) {
b.ReportAllocs()
var out []byte
for b.Loop() {
out, _ = cbor.Marshal(element)
}
benchBytesSink = out
})
}
}plus the symmetric Now the encode benefit is a direct A/B — e.g.:
|
Summary
Felt.MarshalCBOR/UnmarshalCBORused to go through a generic reflection-based CBOR codec.A
Feltis always 4uint64limbs, so this PR encodes/decodes that exact shape directly, and falls back to the generic codec for anything else.Benchmark
Single Felt, no DB:
Real mainnet blocks through pebble:
n=8,p≤0.01everywhere.Allocation count unchanged (1 for Marshal, 0 for Unmarshal). Byte size unchanged except tiny Marshal inputs, which now allocate a fixed 48-byte buffer instead of one sized to the value.
Testing