Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON: support custom encode/decode without intermediate objects #35693

Open
kevmoo opened this issue Jan 17, 2019 · 16 comments
Open

JSON: support custom encode/decode without intermediate objects #35693

kevmoo opened this issue Jan 17, 2019 · 16 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-convert type-enhancement A request for a change that isn't a bug

Comments

@kevmoo
Copy link
Member

kevmoo commented Jan 17, 2019

The current support for custom encoding (toEncodable param and supporting toJson on classes) and decoding (reviver) deal with JSON literal types (String, num, Map, List, etc) requiring intermediate representations be created for all complex objects.

Propose to add three members (see below) to dart:convert and then update the JsonCodec, JsonEncoder, JsonDecoder classes to support then as alternatives to toEncodable and reviver.

Note: I don't expect devs to hand-write implementations of these types. Instead, code generators – such as json_serializable – could be updated to target the new API.

typedef WriteJson = bool Function(Object source, JsonWriter writer);

// Inspired by SDK code
// https://github.com/dart-lang/sdk/blob/e7bd3edc9f/sdk/lib/convert/json.dart#L513
abstract class JsonWriter {
  void writeObject(Object object);

  void startMap();
  void writeKeyValue(String key, Object value);
  void endMap();

  void startList();
  void writeListValue(Object value);
  void endList();
}

// Note: already exists in the SDK sources
// https://github.com/dart-lang/sdk/blob/e7bd3edc/runtime/lib/convert_patch.dart#L79-L101
abstract class JsonReader<T> {
  void handleString(String value);
  void handleNumber(num value);
  void handleBool(bool value);
  void handleNull();
  void objectStart();
  void propertyName();
  void propertyValue();
  void objectEnd();
  void arrayStart();
  void arrayElement();
  void arrayEnd();
  T get result;
}

Related issues:

See https://github.com/kevmoo/json_alt

@kevmoo kevmoo added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-convert type-enhancement A request for a change that isn't a bug labels Jan 17, 2019
@kevmoo
Copy link
Member Author

kevmoo commented Jan 17, 2019

CC @rakudrama @lrhn – I've been able to get much faster encode/decode of custom types vs the VM – imagine if someone who knew what they were doing tried!

@zoechi
Copy link
Contributor

zoechi commented Jan 18, 2019

cc @davidmorgan would that work well with built_value as well?

@davidmorgan
Copy link
Contributor

Yeah, this could be a significant improvement for the VM.

IIUC for web it'd have to be a parser implemented in js instead of the native one, curious if the slowdown makes it worth it overall.

built_value by default uses lists instead of maps in the serialization format, on the assumption that it's cheaper (CPU/RAM) to allocate lists than maps. It's clearly better to allocate neither, if possible...

@mraleph
Copy link
Member

mraleph commented Jan 18, 2019

And if we combine this with dart:mirrors and we get a nice generic JSON deserialization support that does not require build step:

https://mrale.ph/blog/2017/01/08/the-fear-of-dart-mirrors.html

@kevmoo
Copy link
Member Author

kevmoo commented Jan 18, 2019

And if we combine this with dart:mirrors and we get a nice generic JSON deserialization support that does not require build step:

Sure. It's great for both scenarios. All good.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 18, 2019

Thoughts @lrhn ?

@lrhn
Copy link
Member

lrhn commented Jan 21, 2019

Definitely possible. Likely something we can add in a package. I don't expect to change the platform JSON parser now (too many things depending on it). Since the platform JSON parser is written in Dart, there is no reason to think this can't be as efficient.

We might get some code sharing by adding it to the SDK libraries.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 22, 2019

Since the platform JSON parser is written in Dart, there is no reason to think this can't be as efficient.

There are a number of patches that separate the JS and VM implementations. There is also a one-off implementation of parseDouble. It'd be nice if we could build on top of this...and let folks avoid adding a package import.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 22, 2019

CC @leafpetersen who has been looking at improved JSON (de/en)code

@lrhn
Copy link
Member

lrhn commented Jan 28, 2019

I'm trying to abstract our current JSON parser into something like this, where users can supply event callbacks.

The first realization is that it will not work using the native JSON parsing of the browser. There is no way to make those parsers provide similar events. Even using JSON.parse with a reviver gives events that are not in a useful order, and it still builds all the maps and lists anyway.

Using a copy our VM implementation (preferably after fixing a few bugs related to integer overflow), it is fairly easy to do the event callbacks, and I have written listeners that build the current JSON structure, or that build a tightly typed JSON structure (where a [1, 2] would be parsed into a List<int>).
You can also generate events from a Dart object structure, similarly to jsonEncode, and then pass the events, with the same event API, to a listener that does something with it, like creating a string.

All in all, I think it's fairly pretty, but it does copy the majority of the VM JSON implementation in order to get there. Since we can't get the same functionality natively in the browser, I see no better alternative.
Perhaps a solution would be to use the JS JSON parser first, and then traverse the JS structure to generate the events (which still avoids converting the JS structure to Dart maps first).

@rakudrama
Copy link
Member

@lrhn Calling JSON.parse and walking the raw JavaScript to generate the events might have better performance than parsing from scratch, and it is always better to have a smaller solution - we don't want all web apps using JSON to grow at all. The callbacks could have an API that matches walking the JavaScript JSON object, e.g. providing additional information, like transferring ownership of the extracted Object.keys list.

I think for the web experience there needs to be a asynchronous version of the API. Parsing 20MB of JSON takes longer that 16ms, so an async version would be necessary to avoid janky apps.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 30, 2019

FYI: Our friends at Microsoft have a nice layered model here https://github.com/dotnet/corefx/blob/master/src/System.Text.Json/roadmap/README.md

@jonasfj
Copy link
Member

jonasfj commented Feb 12, 2019

CC @sigurdm, @szakarias, this might be relevant when handling jspb in the protobuf package.

@sigurdm
Copy link
Contributor

sigurdm commented Feb 12, 2019

Yes - exciting - I think an event-based json-parser would enable us to improve protobuf json-decoding (especially on the web).

@lrhn
Copy link
Member

lrhn commented Oct 2, 2020

Try package:jsontool for something like this. It has a pull-based JSON reader and a generalized JSON sink for writing.

@gnprice
Copy link
Contributor

gnprice commented Jul 30, 2024

package:jsontool looks neat! I read the planets example and skimmed the JsonByteReader implementation, and the API makes a lot of sense.

For me the key ingredient I'd need before I could switch to it would be something to generate the readJson methods. (Extra helpful would be to do so from package:json_annotation annotations, or something very similar, to ease the migration.) For a whole fleet of classes with a bunch of fields each, I don't really want to write those methods by hand… and I definitely don't want to review PRs that write them by hand, or that manually keep them updated when modifying the types. So I'd love to see something that does that. Perhaps a macro?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-convert type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

9 participants