KEMBAR78
move to direct decoding instead of mapstructure by bobcallaway · Pull Request #2598 · sigstore/rekor · GitHub
Skip to content

move to direct decoding instead of mapstructure #2598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Sep 2, 2025
Merged

move to direct decoding instead of mapstructure #2598

merged 4 commits into from
Sep 2, 2025

Conversation

bobcallaway
Copy link
Member

mapstructure is convenient for clients who may not want to encode the specific type logic into their applications; however it is expensive (uses reflection and lots of allocations) - and the server-side code can do much, much better:

  | Type          | Benchmark                     | Operations/sec       | ns/op          | B/op         | allocs/op   |
  |---------------|-------------------------------|----------------------|----------------|--------------|-------------|
  | hashedrekord  | DecodeEntryMapstructure       | 87,224               | 12,456         | 3,536        | 130         |
  |               | DecodeEntryDirect             | 4,241,101 (+4,764%)  | 281.5 (-97.7%) | 184 (-94.8%) | 10 (-92.3%) |
  |               | DecodeEntryMapstructureMemory | 99,608               | 12,099         | 3,496        | 128         |
  |               | DecodeEntryDirectMemory       | 8,967,894 (+8,904%)  | 139.4 (-98.8%) | 144 (-95.9%) | 8 (-93.8%)  |
  | rekord        | DecodeEntryMapstructure       | 66,650               | 16,335         | 4,912        | 171         |
  |               | DecodeEntryDirect             | 1,998,006 (+2,897%)  | 605.4 (-96.3%) | 352 (-92.8%) | 18 (-89.5%) |
  |               | DecodeEntryMapstructureMemory | 75,574               | 16,184         | 4,784        | 165         |
  |               | DecodeEntryDirectMemory       | 5,949,034 (+7,773%)  | 197.4 (-98.8%) | 224 (-95.3%) | 12 (-92.7%) |
  | alpine        | DecodeEntryMapstructure       | 81,751               | 13,360         | 3,910        | 137         |
  |               | DecodeEntryDirect             | 3,805,863 (+4,555%)  | 317.0 (-97.6%) | 208 (-94.7%) | 10 (-92.7%) |
  |               | DecodeEntryMapstructureMemory | 90,792               | 13,049         | 3,870        | 135         |
  |               | DecodeEntryDirectMemory       | 8,341,514 (+9,087%)  | 145.5 (-98.9%) | 168 (-95.7%) | 8 (-94.1%)  |
  | jar           | DecodeEntryMapstructure       | 68,284               | 16,101         | 4,336        | 165         |
  |               | DecodeEntryDirect             | 3,131,325 (+4,485%)  | 385.4 (-97.6%) | 256 (-94.1%) | 13 (-92.1%) |
  |               | DecodeEntryMapstructureMemory | 72,627               | 15,869         | 4,272        | 162         |
  |               | DecodeEntryDirectMemory       | 7,179,967 (+9,785%)  | 167.9 (-98.9%) | 192 (-95.5%) | 10 (-93.8%) |
  | cose          | DecodeEntryMapstructure       | 67,878               | 15,771         | 4,888        | 160         |
  |               | DecodeEntryDirect             | 2,927,893 (+4,214%)  | 410.6 (-97.4%) | 296 (-93.9%) | 15 (-90.6%) |
  |               | DecodeEntryMapstructureMemory | 80,606               | 15,163         | 4,808        | 156         |
  |               | DecodeEntryDirectMemory       | 6,571,891 (+8,053%)  | 186.7 (-98.8%) | 216 (-95.5%) | 11 (-92.9%) |
  | dsse          | DecodeEntryMapstructure       | 52,696               | 21,506         | 6,184        | 203         |
  |               | DecodeEntryDirect             | 2,665,147 (+4,960%)  | 459.0 (-97.9%) | 328 (-94.7%) | 16 (-92.1%) |
  |               | DecodeEntryMapstructureMemory | 58,820               | 20,479         | 5,956        | 197         |
  |               | DecodeEntryDirectMemory       | 6,529,576 (+10,997%) | 184.7 (-99.1%) | 224 (-96.2%) | 11 (-94.4%) |
  | helm          | DecodeEntryMapstructure       | 81,764               | 13,241         | 3,896        | 138         |
  |               | DecodeEntryDirect             | 3,409,502 (+4,069%)  | 351.9 (-97.3%) | 200 (-94.9%) | 11 (-92.0%) |
  |               | DecodeEntryMapstructureMemory | 93,044               | 12,649         | 3,856        | 136         |
  |               | DecodeEntryDirectMemory       | 8,024,420 (+8,525%)  | 150.9 (-98.8%) | 160 (-95.9%) | 9 (-93.4%)  |
  | intoto v0.0.1 | DecodeEntryMapstructure       | 103,467              | 10,329         | 3,784        | 110         |
  |               | DecodeEntryDirect             | 3,034,082 (+2,833%)  | 396.7 (-96.2%) | 248 (-93.4%) | 13 (-88.2%) |
  |               | DecodeEntryMapstructureMemory | 120,357              | 9,823          | 3,704        | 106         |
  |               | DecodeEntryDirectMemory       | 8,313,902 (+6,805%)  | 163.0 (-98.3%) | 168 (-95.5%) | 9 (-91.5%)  |
  | intoto v0.0.2 | DecodeEntryMapstructure       | 39,009               | 29,362         | 10,742       | 292         |
  |               | DecodeEntryDirect             | 1,528,224 (+3,818%)  | 783.5 (-97.3%) | 552 (-94.9%) | 26 (-91.1%) |
  |               | DecodeEntryMapstructureMemory | 42,055               | 28,240         | 10,637       | 287         |
  |               | DecodeEntryDirectMemory       | 3,237,895 (+7,599%)  | 363.3 (-98.7%) | 448 (-95.8%) | 21 (-92.7%) |
  | rfc3161       | DecodeEntryMapstructure       | 318,961              | 3,735          | 784          | 38          |
  |               | DecodeEntryDirect             | 12,580,731 (+3,845%) | 95.64 (-97.4%) | 48 (-93.9%)  | 3 (-92.1%)  |
  |               | DecodeEntryMapstructureMemory | 319,934              | 3,661          | 784          | 38          |
  |               | DecodeEntryDirectMemory       | 27,762,674 (+8,579%) | 43.53 (-98.8%) | 48 (-93.9%)  | 3 (-92.1%)  |
  | rpm           | DecodeEntryMapstructure       | 70,752               | 15,532         | 4,534        | 152         |
  |               | DecodeEntryDirect             | 3,824,611 (+5,307%)  | 316.1 (-98.0%) | 208 (-95.4%) | 10 (-93.4%) |
  |               | DecodeEntryMapstructureMemory | 78,793               | 15,114         | 4,494        | 150         |
  |               | DecodeEntryDirectMemory       | 8,314,075 (+10,453%) | 143.8 (-99.0%) | 168 (-96.3%) | 8 (-94.7%)  |
  | tuf           | DecodeEntryMapstructure       | 400,794              | 3,015          | 1,816        | 39          |
  |               | DecodeEntryDirect             | 13,219,346 (+3,197%) | 90.99 (-97.0%) | 32 (-98.2%)  | 2 (-94.9%)  |
  |               | DecodeEntryMapstructureMemory | 395,832              | 2,982          | 1,816        | 39          |
  |               | DecodeEntryDirectMemory       | 32,530,498 (+8,119%) | 37.30 (-98.7%) | 32 (-98.2%)  | 2 (-94.9%)  |

this adds the benchmark tests, additional fuzzers, and type-specific calls.

@bobcallaway
move to direct decoding instead of mapstructure
c93a0f3 
Signed-off-by: Bob Callaway <bcallaway@google.com>
@bobcallaway bobcallaway requested a review from a team as a code owner August 31, 2025 17:33
@bobcallaway
add license headers
20e556d 
Signed-off-by: Bob Callaway <bcallaway@google.com>
@codecov
Copy link

codecov bot commented Aug 31, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 62.05924% with 269 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.01%. Comparing base (488eb97) to head (ad4ecbe).
⚠️ Report is 508 commits behind head on main.

Files with missing lines Patch % Lines
pkg/types/intoto/v0.0.2/entry.go 52.94% 41 Missing and 7 partials ⚠️
pkg/types/cose/v0.0.1/entry.go 53.96% 23 Missing and 6 partials ⚠️
pkg/types/dsse/v0.0.1/entry.go 64.63% 24 Missing and 5 partials ⚠️
pkg/types/helm/v0.0.1/entry.go 59.01% 20 Missing and 5 partials ⚠️
pkg/types/jar/v0.0.1/entry.go 63.79% 16 Missing and 5 partials ⚠️
pkg/types/rekord/v0.0.1/entry.go 66.66% 16 Missing and 5 partials ⚠️
pkg/types/hashedrekord/v0.0.1/entry.go 64.00% 14 Missing and 4 partials ⚠️
pkg/types/intoto/v0.0.1/entry.go 64.00% 13 Missing and 5 partials ⚠️
pkg/types/rpm/v0.0.1/entry.go 67.85% 14 Missing and 4 partials ⚠️
pkg/types/alpine/v0.0.1/entry.go 75.40% 12 Missing and 3 partials ⚠️
... and 2 more
Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #2598       +/-   ##
===========================================
- Coverage   66.46%   26.01%   -40.45%     
===========================================
  Files          92      190       +98     
  Lines        9258    25224    +15966     
===========================================
+ Hits         6153     6562      +409     
- Misses       2359    17839    +15480     
- Partials      746      823       +77
Flag Coverage Δ
e2etests 46.64% <46.82%> (-0.92%) ⬇️
unittests 17.05% <57.54%> (-30.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

haydentherapper
haydentherapper previously approved these changes Sep 2, 2025
View reviewed changes
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do our existing e2e and unit tests have enough coverage to verify if any fields were missed or incorrectly decoded?

@bobcallaway
Add missing fuzzers
64cb78c 
Signed-off-by: Bob Callaway <bcallaway@google.com>
@bobcallaway
gofmt
ad4ecbe 
Signed-off-by: Bob Callaway <bcallaway@google.com>
@bobcallaway bobcallaway merged commit 58c9f25 into sigstore:main Sep 2, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haydentherapper haydentherapper haydentherapper approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bobcallaway @haydentherapper