Skip to content

serde2: use our own json#635

Draft
lucix-aws wants to merge 6 commits intofeat-serde2from
feat-serde2-fastjson
Draft

serde2: use our own json#635
lucix-aws wants to merge 6 commits intofeat-serde2from
feat-serde2-fastjson

Conversation

@lucix-aws
Copy link
Copy Markdown
Contributor

I had benchmarked this after merging it and the old (new old) deserializer performance was pretty bad, basically because of json.Token being an interface. It was some odd ~15% allocation increase across the board.

Thus we're just going to toss that and roll our own.

Dave Cheney has a pretty in-depth article about fast json parsing https://dave.cheney.net/paste/gophercon-sg-2023.html which I've largely followed for this. I didn't make every single optimization that he did though.

Running this against the deserialize stuff in jsonrpc10 we're now at a slight performance boost over the old static version across the board, I've made results available at https://gist.github.com/lucix-aws/ba3ca20113b23752144ad25e0f237a58.

lucix-aws and others added 5 commits March 5, 2026 13:13
* implement awsjson10 minus event streams

* forgot about sparse

* Update serde.go

Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>

* Update serde.go

Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>

* idk what this was about

* drop the old todo stuff

---------

Co-authored-by: Luis Madrigal <599908+Madrigal@users.noreply.github.com>
@lucix-aws lucix-aws requested review from a team as code owners March 6, 2026 20:53
@lucix-aws lucix-aws changed the base branch from main to feat-serde2 March 6, 2026 20:54
@Madrigal
Copy link
Copy Markdown
Contributor

Madrigal commented Mar 6, 2026

I'm wary of maintaining our own JSON decoder. I know we like no dependencies, but jumping at 2 AM to fix a parsing issue sounds like a nightmare. What testing do we plan to have before releasing this to ensure it fulfills our needs and handles errors gracefully?

@lucix-aws
Copy link
Copy Markdown
Contributor Author

IMO the most reasonable thing to do would be to just port the tests, or the relevant parts of them, from the stdlib's encoding/json/decode_test.go.

There are alternatives FYI:

  • we could basically just copy in struct scanner from scanner.go and its surrounding implementation. That does more or less what this does. I'm not sure how much json.Token is baked into that, would probably require modifications.
  • we could leverage encoding/json/v2, more specifically encoding/jsontext. The timing is a little awkward here obviously since it seems mostly on the cusp of releasing. We'd have to copy the code in until we moved to go1.25 at a minimum.
  • Use another 3p dep (is there even one?).

Using the existing v1 one is just not an option because of how json.Token works there.

I am generally in favor of us owning our serializers, especially in this world where we are basically redefining the entire SDK in terms of our own end-to-end serde pipeline. We already had a json encoder and a full cbor codec prior to this. encoding/json/v2 (again more specifically encoding/jsontext) does look good though, it's doing some additional things I'm not doing here like string pooling, and I could see us landing on that long-term.

This is going to probably become a point of discussion for xml as well. I suspect when I go to implement that it will affect how I feel about the above point, I'm not sure how well its API fits into what we're doing here. Query/cbor we are covered since we already stabilized on those.

@Madrigal
Copy link
Copy Markdown
Contributor

Madrigal commented Mar 9, 2026

IMO the most reasonable thing to do would be to just port the tests, or the relevant parts of them, from the stdlib's encoding/json/decode_test.go.

YES! that would make me feel way more comfortable owning this. I had some time to think and I warmed up to the idea, we could do interesting things owning the JSON decoder.

Use another 3p dep (is there even one?).

There are a handful of them benchmarked by the people working at JSON v2 here.

@lucix-aws
Copy link
Copy Markdown
Contributor Author

we talked about this offline this morning, we're okay with shipping this if we need to behind the appropriate tests, but we don't want to write off the possibility of using json/v2 (now or in the future) since it seems there are a lot of improvements there.

mostly it is important that we're aware that the current stdlib json solution is a performance regression when applied to our use case and it's something we need to address before shipping

since there's plenty of other work to be done for schema-serde (serde2) I'm going to leave this up for the moment but carry on with implementing the other protocols and event streams, we don't have to make a decision right now

@lucix-aws lucix-aws changed the title serde2: use our own json [hold] serde2: use our own json Mar 9, 2026
@lucix-aws lucix-aws changed the title [hold] serde2: use our own json serde2: use our own json Mar 9, 2026
@lucix-aws lucix-aws marked this pull request as draft March 9, 2026 15:21
@lucix-aws
Copy link
Copy Markdown
Contributor Author

relates #458

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