-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bad performance of /camp endpoint #2569
Comments
@carlobeltrame is working on this i think |
If I understood correctly, @carlobeltrame was working on the idea on how to specify serialization groups via request, which controls what data will be embedded in the response. However, the above behavior I observed even when no data was embedded. First indication seems to be that this something Doctrine related and not API-platform related, but I might be wrong on this. |
That is exactly right. Since I noticed that fixes like #2571 are not possible everywhere (e.g. the picasso can only load a list of schedule entries but not all their content nodes via a simple filter, so we either have to embed the content nodes or we have an n+1 problem), I started working on user-specified serialization groups. |
Trying to further pinpoint on this, I believe the issue comes form the OnetoOne-Relation between The additional queries are triggered by the same code line here: It's documented by Doctrine as a performance issue (see chapter "Performance impact" on below link): |
Wow. This means that for performance it would actually be better if we eliminated the static, non-editable column layout at the root of the content node tree, and changed the relation activity <-> contentNode to a oneToMany relation. But that might make the logic more complicated in different places instead, not sure. |
I struggle a bit to understand why a many-to-one & one-to-one relation has this impact, but a one-to-many relation should be ok. Maybe a one-to-one could also work by changing the owning side. Although in our case both Probably worth experimenting a bit to see what works and also if it really makes a change (or whether the performance impact comes from somewhere else). Options I see:
|
Other options:
|
Trying to dig a bit further down the rabbit hole. As to my current understanding, the following examples could lead to additional SQL-queries and therefore to a potential performance problem. Bidirectional OneToOne (from inverse side; no inheritance)Even without inheritance, bidirectional OneToOne-relations cannot be lazy loaded from the inverse side (the side without foreign-key). OneToOne (owning side) and ManyToOne, if target entity is an abstract class (inheritance)This case is similar to the one above. In this case, doctrine has a foreign-key, but it doesn't know the class type of the target entity without loading it. So doctrine knows whether it should create a Proxy or not, however it doesn't know which type of Proxy to create. OneToManyOneToMany is never a problem (both with or without inheritance), because doctrine always creates an empty collection, independent of whether there are any items in the collection (and also which type of entities). The collection serves as the proxy in this case. Workarounds & solutionsCommunity discussed workarounds/solutions (other than the ones already discussed above). For the OneToOne-relation:
For the ContentNode-Tree:
|
For the content node tree, maybe we could also use nested set instead of parent relations. This is a much more efficient way of storing tree data structures in relational databases, once the initial boilerplate code is written to transform e.g. ->children calls into the correct SQL. There is already an implementation of this in doctrine extensions nestedset. Not 100% sure, but that might help with the eager loading problems, since the relations are not normal doctrine relations anymore. N.B. in hitobito, the group hierarchy is also implemented using a nested set implementation from an external package, and there it works great. |
As discussed try to make option 1 mergeable with following changes:
|
Can we close this issue? |
Code branches
See linked pull requests for timing measurements.
Implemented:
Implemented:
Notes on identified performance problems
Doctrine lazy loading issues
AbstractContentNodeOwner->rootContentNode (Problem: *ToOne to abstract entity)
ContentNode ->owner (Problem: OneToOne inversed not lazy loadable)
Eager loading issues
$propertyMetadata->isReadableLink()
is not true. However, in many cases we setreadableLink
not on the property itself but on a getter (e.g.Camp->getEmbeddedPeriods()
). In this case, data is not eager loaded although the data will be embedded later in the response.Normalization
RelatedCollectionLinkNormalizer
fully normalizes the object before replacing the links. In many cases we load many child entities which are not needed, because we replace them later with a single link (especially for collections, where the child entities are normally not embedded).ContentNode endpoint
The ContentNode endpoint is sill the slowest, as individual content is loaded via separate queries after the ContentNode collection is loaded. Mainly:
Potential solutions:
Initial issue description
Loading a single camp from the API takes much longer than it should (one of the slowest endpoints). There are a lot of SQL request triggered in the backend. Not entirely sure why. Assuming it's connected to eager loading and somehow doctrine tries to manually load the
rootContentNode
of each category and each activity of the camp (even when no such data is embedded).Couldn't really figure out how to avoid.
Doctrine debug log
The text was updated successfully, but these errors were encountered: