perf(remix-server-runtime): use json.parse for route manifest

[redabacha]

Closes: #

• Docs
• Tests

Testing Strategy:

used JSON.parse in the generated code for adding the route manifest as it's faster than using an object literal, see https://v8.dev/blog/cost-of-javascript-2019#json

created benchmarks here: https://github.com/redabacha/remix-manifest-json-parse. it's anywhere from 1x to 2x faster depending on the number of routes in the manifest (the more routes the larger the performance difference from what i've observed).

Creating test manifest with 100 route(s)
cpu: AMD Ryzen 9 5950X 16-Core Processor
runtime: node v18.14.2 (x64-linux)

benchmark               time (avg)             (min … max)       p75       p99      p995
---------------------------------------------------------- -----------------------------
with JSON.parse     461.36 µs/iter   (391.85 µs … 3.36 ms)  460.6 µs   1.19 ms   1.84 ms
without JSON.parse  591.14 µs/iter   (536.34 µs … 3.62 ms)  593.3 µs 831.95 µs   1.07 ms

summary
  with JSON.parse
   1.28x faster than without JSON.parse

[changeset-bot]

🦋 Changeset detected

Latest commit: 4739fbf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages

Name	Type
@remix-run/server-runtime	Patch
@remix-run/cloudflare	Patch
@remix-run/deno	Patch
@remix-run/dev	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
create-remix	Patch
@remix-run/css-bundle	Patch
@remix-run/architect	Patch
@remix-run/express	Patch
@remix-run/netlify	Patch
@remix-run/serve	Patch
@remix-run/testing	Patch
@remix-run/vercel	Patch
remix	Patch
@remix-run/eslint-config	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

This PR has been automatically marked stale because we haven't received a response from the original author in a while 🙈. This automation helps keep the issue tracker clean from issues that are not actionable. Please reach out if you have more information for us or you think this issue shouldn't be closed! 🙂 If you don't do so within 7 days, this PR will be automatically closed.

[machour]

@redabacha Would you mind rebasing your PR and fixing the conflicts ? 🙏🏼

[redabacha]

@redabacha Would you mind rebasing your PR and fixing the conflicts ? 🙏🏼

done ✔️

[brophdawg11]

Wrapping this in single quotes isn't guaranteed to work if the object contains any single quotes. We should switch this over to a double JSON.stringify like we do in react-router-dom StaticRouterProvider.

[brophdawg11]

Updated in 4739fbf

[brophdawg11]

The only concern with the double JSON.stringify is the added bytes over the wire due to the \\ escaping though - but the manifest oughta be cached to alleviate that after the first load?

[pcattori]

JSON.parse vs JSON.stringify is not a bottleneck and only makes sub-millisecond improvement, but adds additional indirection. Feel free to reopen if this becomes a bottleneck.

[brophdawg11]

Just as a note, I don't think we can fully trust the benchmarks reported here since they're from Node on a 16-core machine. The thing aiming to be sped up here is the cost of reading the manifest in the browser on the client device, not necessarily a 16-core node CPU. I would expect to see better results on a slower device/CPU, but would want to see real RUM data before declaring this approach as "better".