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

code splitting / chunking of cache.json files #317

Closed
1 of 5 tasks
thescientist13 opened this issue Apr 11, 2020 · 11 comments
Closed
1 of 5 tasks

code splitting / chunking of cache.json files #317

thescientist13 opened this issue Apr 11, 2020 · 11 comments
Assignees
Labels
enhancement Improve something existing (e.g. no docs, new APIs, etc) v0.7.2

Comments

@thescientist13
Copy link
Member

thescientist13 commented Apr 11, 2020

Type of Change

  • New Feature Request
  • Documentation / Website
  • Improvement / Suggestion
  • Bug
  • Other (please clarify below)

Summary

While code splitting helps with chunking horizontally across routes, for a full graph with hundreds or thousands of pages, a call to the graph would result in huge cache.json file.

We need to think about how to split this up, so cache.json file can be chunked too.

Details

@thescientist13 thescientist13 added the enhancement Improve something existing (e.g. no docs, new APIs, etc) label Apr 11, 2020
@thescientist13 thescientist13 added this to the MVP milestone Apr 11, 2020
@thescientist13
Copy link
Member Author

@hutchgrant
I think it would be good to edit the description here with what you had shared in Slack. Added this issue to project 5.

@hutchgrant
Copy link
Member

hutchgrant commented Apr 11, 2020

Yes I agree. This is a concern we will need to address or scaling larger than 300 pages or using too much data(too many large queries) on <100 could cause problems with slow page load times.

@thescientist13
Copy link
Member Author

I wonder if something like unified would be helpful with either this or #305 ?

@thescientist13
Copy link
Member Author

thescientist13 commented Apr 20, 2020

Tweet from Dan Abramov

But about a typical app architecture. Such as a fully client side app. Or a server-rendered app with a poor approach (like a giant JSON blob in HTML blocking interactions).

😬

@thescientist13
Copy link
Member Author

thescientist13 commented Apr 24, 2020

Docs page transfers 2.6MB of JSON from cache.json and multiple times as well... 😞
Screen Shot 2020-04-24 at 4 34 53 PM

@thescientist13 thescientist13 added the P0 Critical issue that should get addressed ASAP label Apr 24, 2020
@hutchgrant
Copy link
Member

hutchgrant commented Apr 24, 2020

The deepmerge within serialize.js is where we're getting this large cache from. There are 2 queries in the app-template (one isn't necessary), one in the header, and one in the shelf. That's how you have so many requests being made. Also I don't know if our hydration logic is correct either.

In any event, the deepmerge approach can be tweaked so that maybe we dont include 8 pages worth of cached queries into a single cache.json file?

@hutchgrant
Copy link
Member

hutchgrant commented Apr 24, 2020

    const response = await Promise.all([
      await client.query({
        query: ConfigQuery
      }),
      await client.query({
        query: GraphQuery
      })
    ]);

That graph query is massive and when each page is serialized that will be added to every single cache. It's only needed once on initial page load for SPA and could arguably be filtered in the resolver instead. Similarly the header/shelf only need to be queried once. Therein lies another problem.

Many different optimizations are needed here.

@thescientist13
Copy link
Member Author

thescientist13 commented Apr 29, 2020

Some thoughts on quick wins here

  1. Fix overfetching by caching in the client. (but would also be good to know why this is happening)
  2. Reduce duplication of query results within a single cache.json file (e.g. about/cache.json) because deepmerge just keeps doubling up the previous file contents, resulting in duplicates over and over again.

For something like docs/cache.json, this could bring the size down to 35k! We should also try and add a "budget" spec for this to make sure the size can't explode again without it failing at least.


Later down the road, we should look to reduce duplication of query results across all cache.json files, e.g. (about/cache.json, docs/cache.json)

@thescientist13
Copy link
Member Author

Moving the conversation to #345 for the performance and optimizations solutions. The original intent of this issue is too far ahead of our immediate needs right now so don't want to conflate these things too much.

@thescientist13 thescientist13 removed the P0 Critical issue that should get addressed ASAP label May 3, 2020
@thescientist13 thescientist13 modified the milestones: MVP, VP May 3, 2020
@thescientist13
Copy link
Member Author

Another thought with this, like for the graph, is to maybe just be able to filter it per route, like we do with menu? This might help cut down a lot on data returned per index.html, but then you might need to save cache.json per route and sub route, instead.

@thescientist13
Copy link
Member Author

Resolved as part of #383 / #371

@thescientist13 thescientist13 removed this from the VP milestone Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve something existing (e.g. no docs, new APIs, etc) v0.7.2
Projects
None yet
Development

No branches or pull requests

2 participants