statamic / eloquent-driver

Provides support for storing your Statamic data in a database, rather than flat files.
https://statamic.dev/tips/storing-content-in-a-database
MIT License
104 stars 71 forks source link

Massive surge in uncached page loads after upgrading to >= 3.1.3 #261

Closed K3CK closed 1 month ago

K3CK commented 3 months ago

We noticed a massive surge in uncached page loads in one of our projects after upgrading to Eloquent Driver >= 3.1.3.

Before we had page load times of around 1.5s, after the upgrade page load times are around 4s.

The project has around 2500 entries in the database.

It seems to be related to the dirty state tracking changes from https://github.com/statamic/eloquent-driver/pull/248, specifically the line https://github.com/statamic/eloquent-driver/pull/248/files#diff-31fcf8a95fd0dbc8da9e8dc7ae856aa51bd921c50a8059e23df7d411737ab65f

Removing the syncOriginal() call brings back the faster load times.

Any idea what's going on?

K3CK commented 3 months ago

Following up on this it looks like the root cause lies in the inefficient handling of structured trees as it is described here https://github.com/statamic/cms/pull/5899 and will be hopefully addressed in v5 (see https://github.com/statamic/cms/pull/9588).

In our case we have more than 2.000 pages in a structured page hierarchy. For some reason the complete tree is built and validated on each page load (could not yet find out why this is the case or why it would even be necessary; btw the tree building and validating even happens if you load an empty page with no output at all).

While this is already not great, the changes introduced in https://github.com/statamic/eloquent-driver/pull/248 for tracking dirty states of entries make things worse as there are more (IMO unnecessary) operations performed on each of the >2.000 pages in the tree on each page load. In particular: The mentioned syncOriginal() call in Statamic\Eloquent\Entries\Entry is calling getCurrentDirtyStateAttributes() in Statamic\Entries\Entry doing an array_merge() operation on each of the >2.000 pages in the tree. While each call takes only a few milliseconds, this adds up when done to >2.000 pages, resulting in the huge surge in page load times in our case.

What I don't get is why the dirty state is queried at all on the initial page load. So I'm wondering if this a bug in the eloquent driver or if I just should wait until the tree handling is optimized in Statamic v5. Any suggestions?

ryanmitchell commented 3 months ago

I don't think this is something we should be optimising for on the eloquent side (it's meant to be a storage driver). I would wait and see what v5 brings, I'm hopeful the performance improvements there will more than offset the difference isDirty() has made to your site.

K3CK commented 3 months ago

Thanks, Ryan. Yes, that sounds sound. Really looking forward to v5 👍

ryanmitchell commented 2 months ago

Now that alpha 1 for v5 is out you could run some tests - there is a statamic-5 branch here, that you can use which provides support.

ryanmitchell commented 1 month ago

Going to close this, feel free to reopen if its still an issue with v5 now that its out.