When it uses Views with caching search results on page load during Field unserializing it calls Index::load() many times, but for since it is config entity run of entity load is not free.
In my case search results page view with 1 index with 25 fields, 3 facets on the page, and 20 search results generated almost 500 calls to Index::load().
Here blackfire report:
Here the code that producing that \Drupal\search_api\Item\Field::__wakeup:
<?php
public function __wakeup() {
// Make sure we have a container to do this. This is important to correctly
// display test failures.
if ($this->indexId && \Drupal::hasContainer()) {
$this->index = \Drupal::entityTypeManager()
->getStorage('search_api_index')
->load($this->indexId);
$this->indexId = NULL;
}
}
?>At least use static variable to avoid not needed calls to Index::load.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3206362-4--field_serialization_index_loads.patch | 13.55 KB | drunken monkey |
Comments
Comment #2
zviryatko commentedComment #3
idebr commentedComment #4
drunken monkeyHuh, wasn’t aware that config entity loads create a new object every single time. Seems like a terrible idea from my point of view, but probably had good reasons.
Anyways, then yes, we definitely should be careful not to do too much index loading. This also undermines the code we specifically put into place in
\Drupal\search_api\Item\Item::setField()to make sure items and their fields always reference the same index object.So, the simplest solution seems to be to change this to let
Itemtake care of handling the serialization of its fields’ index objects. (And, while we’re at it, also fix the serialization of the item itself, which we somehow seem to have forgotten until now.)Patch attached, please review! (No interdiff as there wasn’t really any overlap anymore.)
Most work here, btw, was getting the new tests to pass. The way we had
SerializationTestset up really was unnecessarily complicated, so the best solution in the end was to completely rewrite the test index handling.Comment #5
drunken monkeyComment #6
drunken monkeyAny feedback on this?
Would be great to get a test and/or review from someone here before committing.
Comment #7
drunken monkeyCommitted.