1074591 - Some JSONWriter tweaks

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Nicholas Nethercote [inactive]]

These tweaks have arisen from using JSONWriter more for DMD (bug 1044709).

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Allow JSON collections to be printed on a single line

In DMD, this lets us change block objects from this:

> {
>  "req": 8192,
>  "alloc": "A",
>  "reps": [
>   "B"
>  ]
> },

to this:

> {"req": 8192, "alloc": "A", "reps": ["B"]},

and change stack trace arrays from this:

> "ENf": [
>  "HKI",
>  "HKJ",
>  "HKK",
>  "HKL",
>  "HKM",
>  "HKN",
>  "HKO",
>  "HKP",
>  "HKQ",
>  "HKR",
>  "HKS",
>  "HKT",
>  "HKU",
>  "HKV",
>  "HKW",
>  "HKX",
>  "HKY"
> ],

to this:

> "ENf": ["HKI", "HKJ", "HKK", "HKL", "HKM", "HKN", "HKO", "HKP", "HKQ", "HKR", "HKS", "HKT", "HKU", "HKV", "HKW", "HKX", "HKY"],

Taking a snapshot with Gmail loaded, this reduces output file size by 18% when
sampling is enabled, and 26% when sampling is disabled. It also makes the files
easier to read, IMO.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove Pointer{Element,Property}() from JSONWriter

I'm no longer using these for DMD, and they're easy to emulate with a
preliminary sprintf() combined with String{Element,Property}().

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8497227 [details] [diff] [review]
(part 1) - Allow JSON collections to be printed on a single line

Review of attachment 8497227 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/JSONWriter.h
@@ +235,5 @@
> +  // specified. If a collection is printed in single-line style, every nested
> +  // collection within it is also printed in single-line style, even if
> +  // DefaultStyle is requested.
> +  enum CollectionStyle {
> +    DefaultStyle,

Can we call this "MultiLineStyle" instead?  MultiLineAndIndentedStyle would be even better, if a bit verbose.

Comment 4

[Nicholas Nethercote [inactive]]

> Can we call this "MultiLineStyle" instead?

I chose "DefaultStyle" in an attempt to communicate the idea that it is overridden by any enclosing SingleLineStyle occurrences... it sounds less authoritative. But I can do MultiLineStyle.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Fix unbounded growth in JSONWriter's vectors(!)

Oy, this is a bad one. The JSONWriter vectors are meant to be at least as long
as the maximum depth of array/object nesting; this usually isn't very long. But
the code I wrote increases their size by one every time a new array/object is
opened. This means their size is proportional to the number of arrays/objects
written, not their maximum nesting depth.

I discovered this on B2G, where DMD's JSON output was causing OOMs. Yikes.

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8499257 [details] [diff] [review]
(part 3) - Fix unbounded growth in JSONWriter's vectors(!)

Review of attachment 8499257 [details] [diff] [review]:
-----------------------------------------------------------------

Just think, in a couple years, we wouldn't have caught this OOM on mobile phones, thanks to exponential memory growth...

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7151ebc29075
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eb845ba1fc6
https://hg.mozilla.org/integration/mozilla-inbound/rev/31b722e8e85d

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7151ebc29075
https://hg.mozilla.org/mozilla-central/rev/1eb845ba1fc6
https://hg.mozilla.org/mozilla-central/rev/31b722e8e85d