Skip to content

Fix quadratic decode time with CustomUnmarshaler nodes - #966

Closed
chiliec wants to merge 1 commit into
goccy:masterfrom
chiliec:fix-custom-unmarshaler-quadratic
Closed

chiliec wants to merge 1 commit into
goccy:masterfrom
chiliec:fix-custom-unmarshaler-quadratic

Conversation

@chiliec

@chiliec chiliec commented Oct 23, 2026

Copy link
Copy Markdown

What

Fixes #965: decode time is quadratic in the number of CustomUnmarshaler nodes.

Why

newFormatter walks the entire document token chain to rebuild its
tokenToOriginMap on every call. decodeByUnmarshaler creates a fresh formatter
(via unmarshalableDocument) for each CustomUnmarshaler/BytesUnmarshaler node,
so decoding N such nodes rebuilds the whole-document map N times → O(N²).

The map depends only on the document's token chain, which is the same for every
node in that document, so it can be built once and reused.

Change

  • Extract the map construction into format.NewTokenToOriginMap and let
    newFormatter accept a prebuilt map (new FormatNodeWithTokenOriginMap;
    FormatNodeWithResolvedAlias still works, passing nil).
  • The decoder builds the map once per document (keyed on the token-chain head so a
    reused Decoder across different trees rebuilds correctly) and reuses it for all
    custom-unmarshaler formatting.

Formatter output is unchanged — only the per-node rebuild is removed.

Tests

Added BenchmarkUnmarshalWithCustomUnmarshaler mirroring the issue's repro.

Validation (go1.23.4, go test ./... all pass):

Before (ns per line stays proportional to line count — quadratic):

n=250    101763 ns/n
n=500    199369 ns/n
n=1000   441802 ns/n
n=2000  1100730 ns/n

After (flat — linear):

n=250   ~4465 ns/n
n=500   ~4467 ns/n
n=1000  ~6794 ns/n
n=2000 ~11678 ns/n

At n=2000 the decode drops from ~2.2s to ~23ms. Full suite green on the branch.

newFormatter rebuilt the whole-document token-to-origin map on every call,
and the decoder constructs a new formatter per CustomUnmarshaler node, making
decode time quadratic in the number of such nodes. Build the map once per
document and reuse it across formatter calls.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.65%. Comparing base (edee2f9) to head (ebb7671).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #966   +/-   ##
=======================================
  Coverage   80.65%   80.65%           
=======================================
  Files          22       22           
  Lines        6845     6856   +11     
=======================================
+ Hits         5521     5530    +9     
- Misses        905      906    +1     
- Partials      419      420    +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@orivej

orivej commented Oct 23, 2026

Copy link
Copy Markdown

@chiliec Thank you! Please review #967, I think it is simpler.

@chiliec

chiliec commented Oct 23, 2026

Copy link
Copy Markdown
Author

Closing in favour of #967, which removes the full-document scan entirely and is faster.

@chiliec chiliec closed this Oct 23, 2026
@chiliec
chiliec deleted the fix-custom-unmarshaler-quadratic branch October 23, 2026 13:54
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.

Decode time is quadratic on the number of CustomUnmarshaler nodes

3 participants