Skip to content

go: use UTF8DecodeInvalid in fxamacker/cbor wrapper - #80

Merged
leo-gan merged 1 commit into
leo-gan:masterfrom
fxamacker:fxamacker-cbor-wrapper-review
Aug 12, 2026
Merged

go: use UTF8DecodeInvalid in fxamacker/cbor wrapper#80
leo-gan merged 1 commit into
leo-gan:masterfrom
fxamacker:fxamacker-cbor-wrapper-review

Conversation

@fxamacker

Copy link
Copy Markdown
Contributor

This PR uses a decoding option to disable UTF-8 validation because generated fixtures are valid UTF-8 and trusted. Skipping UTF-8 validation improves decoding speed by 2-12%, depending on string content.

Other changes:

  • correct a comment about CoreDet
  • fix a broken link in a comment
  • add a comment to mention fxamacker/cbor provides toarray and keyasint struct tag options that can further improve performance and reduce encoded size if they are added to Go structs in the benchmark file (model/v2/generate.go)

This commit uses a decoding option to disable UTF-8 validation
because generated fixtures are valid UTF-8 and trusted.
Skipping UTF-8 validation improves decoding speed by 2-12%,
depending on string content.

Other changes:
- correct a comment about CoreDet
- fix a broken link in a comment
- add a comment to mention fxamacker/cbor provides toarray and
  keyasint struct tag options that can further improve performance
  and reduce encoded size if they are added to Go structs in the
  benchmark file (model/v2/generate.go)

@leo-gan leo-gan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review notes:

Change: DecOptions{UTF8: cbor.UTF8DecodeInvalid} skips UTF-8 validation on decode. API exists in the pinned fxamacker/cbor/v2 v2.9.2. Fixtures are generated/trusted, so this is appropriate for a throughput path and matches the library author's recommendation.

Comments: CoreDet note clarification, fixed docs link, and notes about toarray/keyasint (informational only — structs are unchanged).

Scope: Single-file harness tweak; no go.mod change. Local go test ./serializers/ passes on this branch.

Approving and squash-merging.

@leo-gan leo-gan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sensible throughput-path decode option for trusted benchmark fixtures; comments are accurate. LGTM.

@leo-gan
leo-gan merged commit f317e04 into leo-gan:master Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants