search

catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.93k stars 564 forks source link

We have inconsistent coding conventions about to/from JSONing #1961

Open natduca opened 8 years ago

natduca commented 8 years ago

We have a variety of python and javascript code that has to convert to and from JSON. In Telemetry value system, we have a convention, and then in tracing/units, we have another convention. And in tracing/values, we have yet another convention.

We should probably come up with a common convention here that says how to do to/from json in each language, and then do a fixit to normalize.

natduca commented 8 years ago

@dave-2 @anniesullie @eakuefner fyi

natduca commented 8 years ago

@nedn points out, maybe its time for us to go to something more grown up. Protobuf, capn'proto, yaml... @paulirish whats popular these days amongst the cool kids?

benshayden commented 8 years ago

I think I heard that even the fastest javascript implementations of protobufs were still slower than native implementations of json? Would it be possible to add protobufs to the web platform?

nedn commented 8 years ago

@benshayden the context here is serializing the data for values & units, which supposed to be "small data". I think it's unlikely we gonna do s.t non standard for the json trace object.

paulirish commented 8 years ago

From my tests, protobuf, msgpack and ion are 10x to 100x slower than native JSON. both encoding and decoding. :/

I haven't poked at flatbuffers though it may work well for us.

eakuefner commented 8 years ago

Resurrecting this bug because it's coming up in https://codereview.chromium.org/2046553003

Opinions:

  1. Let's shelve discussion of alternative formats for the purposes of this bug because it's a rathole.
  2. AsDict and _AsDictInto (asDict or asDictInto_ in JS syntax) are the right pattern here and we should record it in the style guide.

@zeptonaut

natduca commented 8 years ago

I'm so sorry about this messed up underscore convention, folks. This is all my fault: there was a time when I was coding telemetry python & tracing & c++ at the same time and I compltely lost track of where the underscore goes. Anyway, its definitely a mess, and I'm hugely supportive of doing the right thing here!

And, definitely, lets just encode our to/from json convention we have in the coding convention doc, and land more code :)