50ms JS parse time on non-json pages

[matthewoates]

This extension adds ~50ms of JS execution time for each page. There should be a lightweight script that checks if there is JSON on the page before loading and parsing the larger script.

Note viewer.js is running for 50ms.

[tulios]

Agreed. The original check was much simpler but after a while some people reported some problems and I switched to this https://github.com/tulios/json-viewer/blob/master/extension/src/json-viewer/check-if-json.js#L61

I can try to break down the execution and see if I can reduce the time. Thanks

[matthewoates]

Sweet. Another idea:

Since this runs at the start of every single page load, something like a series of checks sounds best instead of trying to parse the body.

Something like:

function isJSON(str) {
  return str
    && firstCharacterIsOpenBrace(str)
    && isValidJSON(str);
}

The firstCharacterIsOpenBrace check (plus some additional lightweight logic for JSONP support) is a very performant way to short-circuit 99.9% of pages that aren't JSON.

Of course, my first suggestion as opposed to the suggestion in this comment will yield a much larger performance boost.

[zwacky]

Today I saw this as well, that json-viewer uses 7.2% scripting time on a javascript heavy webapp (MacBook Pro).

I like the simplicity in @matthewoates suggestion. Also keep in mind that the execution time will proportionally increase with the size of the page load.

[tulios]

@zwacky @matthewoates sure, I think this should be fixed. I'll try to work on this issue ASAP. Thanks for the input!

[matthewoates]

@zwacky The first check will run in constant time, since you're just scanning for {

[zwacky]

I did a little profiling and the checkIfJson is not the bottleneck here. It is the webpack module loading.

[matthewoates]

Yes. Please read my original comments @zwacky

Of course, my first suggestion as opposed to the suggestion in this comment will yield a much larger performance boost.

[zwacky]

I'm no webpack pro and wanted to point out that __webpack_require__ takes up most of the time.

That's why I posted my screenshot, which might made you think I was ignoring your comment altogether, @matthewoates.

Now I see that there are ten thousands of lines in the viewer.js file.

[matthewoates]

Ah. I'll elaborate on what's going on here. The __webpack_require__ is the first pass of all of the JavaScript, since every module is wrapped with __webpack_require__. The first pass is notoriously slow because it has to be parsed then either has to be just in time compiled or interpreted. Later runs can be much more performant.

tl;dr: You'll save performance by having one light script include the larger script if and only if JSON is detected. I think we're all in agreement here.

[jimmyjxiao]

Maybe this is a stupid question, but can't we just look at the HTTP headers and see if it's application/json to know if the content is JSON?

[tulios]

Hi @zowpowow, in an ideal world yes, but there are lots of APIs returning text/html among other things as content-type. Take a look at issue 108 for more context

[richiksc]

If the JSON is served with an improper content-type, then users can enable it by manually clicking on the exception. I think it is better to give the majority of users a performance boost by supporting the standard instead of fully supporting all edge cases with the same user experience - give them a fallback instead. This aligns with the progressive enhancement model, and it's why we don't often polyfill things to work on IE.