I did this one "off the clock" so I did have a little fun with a PHP Generator ;)

I saw a report that the resource repository was making thousands of DB queries and started to look into it. Fortunately, it looks like updating to the latest version with #2906539: Performance bug with references already took their uncached page loads from dozens of seconds to <10s. But I think we can do better!

In the current code, every call to ConfigurableResourceTypeRepository->get($entity_type_id, $bundle) calls ->getResourceTypes(FALSE), which means that the "cached" resource types are never used, which means the code will always load and process config. This fixes that.

CommentFileSizeAuthor
#4 interdiff-commit.txt4.17 KBe0ipso
resource-type-repository-doesnt-cache-enabled-resources.patch4.92 KBgabesullice

Comments

gabesullice created an issue. See original summary.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Thanks for the report and the patch! And thanks for reminding me that generators are a thing in PHP.

I think generators are cool, but we will never be able to take full advantage of them without an event look like ReactPHP's. For readability reasons (generators are a bit obscure) I've refactored to use regular arrays.

  • e0ipso committed c2d0f53 on 8.x-1.x authored by gabesullice 
    Issue #2941095 by gabesullice, e0ipso: ResourceTypeRepository loads all...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
StatusFileSize
new interdiff-commit.txt4.17 KB
gabesullice’s picture

gabesullice
Primary language English
commented 
+++ b/src/ResourceType/ConfigurableResourceTypeRepository.php
@@ -112,7 +112,8 @@ class ConfigurableResourceTypeRepository extends ResourceTypeRepository {
+      foreach ($this->getEntityTypeBundleTuples() as $tuple) {
+        list($entity_type_id, $bundle) = $tuple;

Here's a nice trick:

foreach ($this->getEntityTypeBundleTuples() as list($entity_type_id, $bundle)) {
}

This is good in PHP 5 too.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Ah nice. I had it like that but I was too lazy to check php5. Thanks for the tip!

Status: Needs review » Needs work

The last submitted patch, resource-type-repository-doesnt-cache-enabled-resources.patch, failed testing. View results

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Issue tags: +Needs reroll
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs work » Fixed
Issue tags: -Needs reroll

Well, that comment didn't belong here.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.