JsonReader/Writer for direct String reading/writing

square / moshi

A modern JSON library for Kotlin and Java.

https://square.github.io/moshi/1.x/

Apache License 2.0

9.69k stars 758 forks source link

JsonReader/Writer for direct String reading/writing #1089

Open bnorm opened 4 years ago

bnorm commented 4 years ago

Every once in a while when using Moshi I need to use the JsonAdaptor.fromJson(String) function. While I've never had any real performance issues with this function, I found it odd to encode the entire UTF-8 string into a okio.Buffer just to read it from that Buffer as UTF-8 again. Since UTF-8 encoding/decoding is not cheap, there might be some significant performance gains to be had here.

I hacked together a working - but very rough - implementation of a JsonReader which reads directly from a java.lang.String and a JsonWriter which writes directly to a java.lang.StringBuilder. Still a lot of refining and benchmarking to do, but I wanted to create this ticket first to see if there was interest in a pull-request along these lines.

JakeWharton commented 4 years ago

Where are these strings coming from?

bnorm commented 4 years ago

A few of the use cases I have had:

The Amazon SQS library returns Messages with the body as a string.
Some database libraries like jasync-sql will return JSON columns as strings.
OkHttp's WebSocketListener.onMessage will return a string for text messages.

JakeWharton commented 4 years ago

That's fair. Also web sockets :sakes-fist: ...

I think the problem here is that it brings quite a lot of complexity for a small gain. We needed the object-based reader/writers to produce a DOM-like model that could be manipulated without concrete types and without streaming. Here, though, we don't strictly need a character-based reader/writer, it becomes "just" an optimization.

One thing we could do is look into making the reader/writer APIs intentionally extensible. This would allow you to supply your own subtypes. Although right now we're using them both as an interface and a place for common shared code and we really would only want to expose the former.

bnorm commented 4 years ago

Agreed, it's not strictly needed, and you make a good point about it increasing complexity. I'll hold off any further work on it for now. If it really becomes a performance problem I can always come back to this optimization later.