Silverstripe-framework: Epic: Too many sites have 100s of queries per pageview

[sminnee]

Just pointing out the elephant in the room here...

Sites with massive numbers of queries – sometimes more than 1,000! – are not all that uncommon. We can say "oh, they're not coded correctly", and that's true, but it happens too frequently for SilverStripe not to share the blame.

I don't think that there's 1 fix for this, but I want to flag that I think this would be worth digging into deeper.

I'm not sure what a reasonable target on a moderately complex site should be, but it should be very rare that more than 100 queries are necessary, and less if a partial cache is warm.

Some thoughts:

• Eager loading
• Per-page query caching
• Better tools for building batch queries (for use-cases such as batch canView calculation)
• Easier logging of query counts to various places

[sminnee]

CC @stojg @jakedaleweb

[tractorcow]

I recall we had a conversation about how we could possibly cache get() calls similarly to how we cache get_by_id and get_one.

$menu = SiteTree::get()->filter('ParentID', 0); // db hit
$footerMenu = SiteTree::get()->filter('ParentID', 0); //  Could this not be cached?

We could use per-query cache controls using query parameters (e.g. so it could be disabled in the CMS).

[sminnee]

Yeah, the other thing about such a cache is that we could potentially implement eager loading (@unclecheese is looking at this) as a system that pre-warms such a cache with the results of 1 or more queries.

In my view I would start with an in-memory, per-record cache (i.e. an array of some sort) of actual PHP objects. An identity map, in other words. This also keeps the cache-invalidation scenarios more straightforward.

[kinglozzer]

Not too long ago I played around with an API like:

$menu = SiteTree::get()->filter('ParentID', 0)->remember(); // Request lifetime
$menu = SiteTree::get()->filter('ParentID', 0)->remember(300); // Cache for 5 minutes

https://github.com/kinglozzer/sapphire/commit/0d9b7f70c1d31b6a18f8fa0138e6534a7df1f19f

I hit issues with serialising database query results, so I think you’re correct that we’d need a cache of actual PHP objects @sminnee.

[sminnee]

Just to be clear, when I say "cache of PHP objects" I mean "an array". And then you don't need any persistence flag because it's just for the current runtime.

I would probably hold off implementing persistent query caches until after this functionality was implemented; I'm less convinced that it's a silver-bullet for performance and I believe that MySQL already has caching functionality built into it.

[tractorcow]

Yeah the better solution is to just not make that many queries. :)

[maxime-rainville]

@unclecheese got an RFC and experimental module for eager loading.

[phptek]

Someone spent a hell of a lot time and energy looking into this (blaming canView() at the time) some years ago...I'll see if I can't dig it up.

[phptek]

This bad boy: https://github.com/silverstripe/silverstripe-framework/issues/2979

[gurucomkz]

I've been working on an Eager Loading plugin that looks quite good now.
I'd appreciate some feedback and collaboration from those who are interested.

[unclecheese]

Note that the current PR for eager laoding solves this on a much more fundamental level, affording an open API for cached queries.

[gurucomkz]

That does look good, and I actually took some inspiration in that work @unclecheese.

However, the PR is too intrusive and I needed an approach that would surely not break the existing functionality.

Perhaps I should take some more inspiration and make my code a lot more API-oriented like yours.
I think the major difference in our approaches is that I simply inject Eager Loading at the DataList::getGenerator() level, that what makes it safer, I think.

[unclecheese]

Maybe redirect this discussion to how we get that PR merged? I think there are only a couple test failures, and maybe a BelongsManyManyLoader that needs to be finished? It's been stale for so long. Mostly owing to my lack of active advocacy. :-)

[gurucomkz]

I don't mind if you let me