Closed Bug 1074591 Opened 5 years ago Closed 5 years ago

Some JSONWriter tweaks

Categories

(Core :: MFBT, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(3 files)

These tweaks have arisen from using JSONWriter more for DMD (bug 1044709).
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.
Attachment #8497227 - Flags: review?(nfroyd)
I'm no longer using these for DMD, and they're easy to emulate with a
preliminary sprintf() combined with String{Element,Property}().
Attachment #8497232 - Flags: review?(nfroyd)
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.
Attachment #8497227 - Flags: review?(nfroyd) → review+
Attachment #8497232 - Flags: review?(nfroyd) → review+
> 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.
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.
Attachment #8499257 - Flags: review?(nfroyd)
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...
Attachment #8499257 - Flags: review?(nfroyd) → review+
You need to log in before you can comment on or make changes to this bug.