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

Page state store caching, preloading, and memory management #204

Merged
merged 18 commits into from
Oct 29, 2022

Conversation

arctic-hen7
Copy link
Member

This adds proper eviction to the page state store, which means pages won't be stored in the state forever. By default, 25 pages with state are allowed. Past that number, the oldest pages will have their states removed. Additionally, this will minimize the number of network requests by avoiding any network requests to pages that already have a state in the PSS (which would have been preferentially used anyway). That should speed up Perseus apps exceptionally, and it's not a breaking change due to the priorities introduced in v0.3.4.

The eviction system is a breaking change however, as it removes the Default implementations for both RenderCtx and PageStateStore, while also adding the eviction system, which may break some uses of freezing/thawing.

This should fix any memory blowouts (not leaks, just the storage of
every single page's state can get a bit heavy), which occasionally
occurred in development.
After a page is fetched as a subsequent load, it won't be fetched again
until the PSS fills up! This doesn't work with initial loads, since the
head is pre-interpolated (and I don't want to increase the bundle size
by doubling it up in a variable; reading from the HTML is unreliable
since JS will likely have already modified it).
@arctic-hen7
Copy link
Member Author

Alright, the system to prevent unnecessary network requests is done! There's one quirk: a page is only 'cached' properly after it's been loaded through the subsequent loads system. In other words, the very first page the user loads won't be properly cached unless they go back to from another page. The reason is that full caching requires having the <head> as a string, which we don't have for that first page, since the head comes pre-interpolated. Reading it from the HTML is risky, since it will probably already have been modified by JS, and I don't want to send it through with a JS variable, because that would potentially increase the size of the first page HTML files dramatically.

Also, the PSS does have a (configurable) maximum size. By default, if a user visits page A, then 25 others, none of which are page A again, the PSS will drop the information for page A from the cache.

This has involved making `RenderCtx` hold much more data in the browser,
like the error pages and render context, though this should make routing
much lighter.
Apparently, it's perfectly valid to pass the Sycamore scope through to
the Sycamore router, which means we an access everything we need from
the render context! (I reckon there'll be a performance improvement to
moving the render context into a dedicated system though, beyond
Sycamore's context.)
This was needed for the website fixes to make the tests pass.
@arctic-hen7
Copy link
Member Author

I've also expanded this to begin building the preloading infrastructure in the client-side systems, which will eventually grow into a system for pages to automatically declare the preloading of other pages.

Note: tests should also be fixed after the merge from main.

I think this led to some confusion the other day, so it's clarified now.
Just that we don't need `G::IS_BROWSER` if we're target-gating as well.
This includes a new `core/preload` example.
@arctic-hen7
Copy link
Member Author

I've now built the user-facing preload system, along with an example to accompany it! You can now preload any page in a Perseus app with the minimal number of network requests, fully ahead-of-time, with a single line of code! (I'll flesh this out fully as mentioned above in a future PR.)

This was for the #212 fix on `examples/.base/`.
And this one was for the #212 fix on the testing script!
This is achieved through an extra `<meta>` delimiter that denotes the
end of the `<head>`, which should be pretty reliable at getting what the
user intended.
Advanced `<head>` manipulations *could* in rare cases lead to bugs, so
the user can turn this off if necessary, and it's documented in the FAQ section.
@arctic-hen7
Copy link
Member Author

Alright, I've enabled initial page caching under an enabled-by-default feature flag by using a <meta> tag to delimit the end of the head (this is created in such a way that it would be very hard for the user to add essential content after it). That can then be used to deduce what the head was and cache it. That should make this ready for merging!

@arctic-hen7 arctic-hen7 changed the title Eviction and automatic fetching from page state store Page state store caching, preloading, and memory management Oct 29, 2022
@arctic-hen7 arctic-hen7 marked this pull request as ready for review October 29, 2022 04:39
@arctic-hen7 arctic-hen7 merged commit 0c4fa6b into main Oct 29, 2022
@arctic-hen7 arctic-hen7 deleted the feat-pss-improvements branch October 29, 2022 20:42
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 this pull request may close these issues.

None yet

1 participant