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

Performance - use JSON.parse while sending the json to browser. #1457

Closed
pajaydev opened this issue Dec 2, 2019 · 4 comments
Closed

Performance - use JSON.parse while sending the json to browser. #1457

pajaydev opened this issue Dec 2, 2019 · 4 comments

Comments

@pajaydev
Copy link
Contributor

pajaydev commented Dec 2, 2019

Description

From recent talks from v8 team, we know that JSON.parse approach is much faster compared to the JavaScript object literal. JSON.parse() is much faster than JavaScript evaluation. I got inspiration from the below article to make this change
https://v8.dev/blog/cost-of-javascript-2019#json

Why

This may be effective when you use handful of stateful components in your marko app and marko ships those to the browser. In the above case, browser takes time to parse and compile JS, I have made a small comparison with a sample JSON file in my machine. Below are the results

Before changes

Screen Shot 2019-12-01 at 9 31 29 PM

After changes

Screen Shot 2019-12-01 at 9 31 16 PM

Possible Implementation & Open Questions

We know that marko uses warp10 internally to serialize the data while sending it to browser. We can make this change as suggested below in this file https://github.com/marko-js/marko/blob/master/src/runtime/components/index.js#L188

      componentGlobalKey +
        "=(window." +
        componentGlobalKey +
        "||[]).concat(JSON.parse(" +
        JSON.stringify(safeJSON(warp10.stringify(renderedComponents))) +
        "))||" +
        componentGlobalKey

Kindly share any thoughts, concerns or feedback.

CC: @DylanPiercey @mlrawlings

Is this something you're interested in working on?

Yes

@tigt
Copy link
Contributor

tigt commented Jan 29, 2020

Related: marko-js/warp10#2

@tigt
Copy link
Contributor

tigt commented Jan 29, 2020

Slightly related: it should be safe to replace the Array#concat() call in your suggested code with Array#push(), right? 2 less characters, but more importantly it won’t return a newly-copied array.

@DylanPiercey
Copy link
Contributor

DylanPiercey commented Jan 29, 2020

@tigt after Marko initializes it actually replaces the global key with an object which has a concat method. Since the scripts generated also assign to the global, we also return the concat object from the concat method call.

See here: https://github.com/marko-js/marko/blob/master/src/runtime/components/init-components-browser.js#L275

@DylanPiercey
Copy link
Contributor

Overall the impact of this in benchmarking seemed to be minimal and there are benefits to our JS based serialization approach (deduping, smaller runtime, more data types, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants