plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
454 stars 613 forks source link

Listing blocks SSR slows down the page rendering #2964

Open cekk opened 2 years ago

cekk commented 2 years ago

Describe the bug

I'm trying Volto 14 on a big website (yes, it's always the same) and i've found that with listing blocks' ssr enabled (getAsyncData) the ssr page took a lot of time to be rendered (~ 12 seconds instead of ~2 with listing blocks' ssr disabled).

The user have to wait for the ssr page and then the client-side rendering (that made the same requests) to have a working page.

Is there anything that we can do to avoid this double rendering/requests?

Ok, we could cache query responses with varnish somehow, but is there something that we can do on Volto? Maybe enable the SSR only for robots or using a pre-populated store in the client (populated by the ssr) so we don't need to fetch all data from the backend twice.

Software (please complete the following information):

tiberiuichim commented 2 years ago

@cekk Is this with fullobjects or with the summarized listing?

cekk commented 2 years ago

With summarized

mtoepfl commented 2 years ago

​+1 for going with React hydration and preloaded state, like it's documented here:

https://redux.js.org/usage/server-rendering#the-client-side

In one of our projects it was also a game changer in Developer Experience!

Edit: Sorry, I missed that there is already window.__data, so I did not get the point, right? It's getAsyncData?

tiberiuichim commented 2 years ago

@mtoepfl We already have hydration of React components and redux state with Volto.

The problem is like this:

@cekk: You can override the listing block's getASyncData with a method that returns a simple empty promise. This would remove the serverside rendering of listing blocks. Some other options would be, for example, to make getAsyncData smarter, to look into the listing block data, so that the SSR can be enabled/disabled from the block.

But, as you suggest, the double request problem should be solved. I'm not 100% sure that the redux data is hydrated in a way that's compatible with the rest of the client-side code (matching URLs as @id, for ex, etc.

mtoepfl commented 2 years ago

Thanks for clarification. Allow me another question to the getAsyncData mechanism:

In https://github.com/plone/volto/blob/d604140d293fef52566a88de1f4906687269f604/src/components/manage/Blocks/Listing/getAsyncData.js#L5 redux-dispatch-function is called, so after SSR all requested data should be in window.__data and so in client-side initiale redux-store already, right? So why fetchContent is needed on client-side before first hydration?

tiberiuichim commented 2 years ago

So why fetchContent is needed on client-side before first hydration?

It's not needed, you are correct. The fix would be to make this smarter. The querystringsearch reducer would have to keep all the results per query params, so that they can be checked in the redux store and then a query can be triggered appropriately. It wouldn't be ideal and some other form of "invalidation" may be needed. Perhaps only trigger for the case when the start is 0, then immediately trigger an invalidation of that, or maybe mark the SSR result in a special way... either way, it gets more complex and in the end, it's just an optimization.

I'd be more interested in understanding why the catalog result takes so long to generate, I think it's a "healthier" thing to pursue.

cekk commented 2 years ago

@tiberiuichim yes, i've overrided getAsyncData right now and things get better.

For what i've seen, the problem isn't a single query to the catalog because if i try one single endpoint call, it's fast. The problem is that in my homepage i have several listing blocks and there are ~10/15 @querystring-search (plus other standard enpoint calls that trigger something in the catalog too) calls in parallel, and some of them took ~20/25 secs.

I have the same situation (homepage with several listing blocks) also in other (lighter) volto sites (with the same products installed), but there i don't have these numbers. The main difference is the catalog dimension (some k of objects versus 300k).

tiberiuichim commented 2 years ago

@cekk Maybe optimize ZODB cache params so that the catalog fits in RAM? If catalog queries are that slow, it's a problem.

sneridagh commented 2 years ago

We hit that too, and had to disable it. It's not that the code is wrong. We need to come up with a solution of the paradigm of landing pages that could have a lot of listings, a lot of teasers, and blocks that might want to pull data from the database in multiple approaches.

/cc @tisto

tiberiuichim commented 2 years ago

@sneridagh I propose a "volatile" @component in restapi response that stashes data meant to be read by each block (in the frontend). And a subscriber-based block-transformer like mechanism for each block to write its own "volatile" data.

At some point I've proposed the _v_* volatile block transformer, which could be used for the same purpose (I use them for slots). The downside is that there needs some code in Volto that goes through the current content data (in the form submission) and strips it, so that it doesn't get sent to the server. It feels dirtier.

sneridagh commented 2 years ago

Interesting approach!

@ericof proposed me a similar solution for dealing with vocabularies in brain restapi responses not long ago, and he's going to give it a try soon.