Conversation
* 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>
|
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? |
|
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:
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. |
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.
There are a handful of them benchmarked by the people working at JSON v2 here. |
|
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 |
|
relates #458 |
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.