Fix entity selector gen + massive perf improvements

[cconard96]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#16341

Issue:
When an entity is created/updated/deleted, the 'entity_selector' cache entry is deleted, but a recent patch changed the entity selector to use a cache entry with a calculated key based on the user's current profile entities.

$ckey = 'entity_selector'
    . sha1(json_encode($_SESSION['glpiactiveprofile']['entities']))
    . sha1($base_path) // cached value contains links based on `$base_path`, so cache key should change when `$base_path` changes
;

So, the entity selector is not correct after an entity is added/changed if the cache entry was already set .

Solution:
Don't cache entity selector data in a key based on the active profile's entities. Instead, rework the getTreeForItem function to utilize the existing getSonsOf function which makes use of the sons cache in GLPI_CACHE and at the DB level if present. Then after we have every child's ID, we can get all of the names and parent IDs in a single DB request.

Result:
For me on my GLPI with 38 entities, the time required to generate the selector went from 7ms to 4ms with this PR removing the use of the combined selector cache entry and replacing it with the use of the sons_cache entries. Indeed this is still longer than it would have taken to get the full selector data from the cache, but that doesn't seem possible without using a single cache entry again. Not sure about the effect on performance with a massive number of entities, but I suppose performance could be improved more if the entity parents/names are cached in some way.

Also, caching data based on the $base_path doesn't seem to be needed as the base path isn't saved in the data. Instead it is added after the entity selector data is already fetched.

[tsmr]

I confirm. There is a problem in 10.0.11. After create a new entity, this entity is not find by search engine

[cconard96]

I confirm. There is a problem in 10.0.11. After create a new entity, this entity is not find by search engine

If you have time, can you test this PR to validate the fix as well as check the impact on performance?

[tsmr]

I have applied your fix on a client instance, it's ok (but i haven't check performance)

[orthagh]

I think prefer this solution over #16306.
If everyone agree to choose it, and to avoid any further issue, a new test adding/removing entities and compare somehow (I'm not sure how to do this in the current state) to the rendered tree. The entity selector is a crucial part and we should avoid regressions here in priority.

[cedric-anne]

Since #15792, the entity tree is loaded only on demand, so, as long as generating the HTML is not anymore done ion every page display, it seems acceptable to remove the cache from here.

I just wonder how long it would take to generate a tree with thousands of entities.

[orthagh]

I just wonder how long it would take to generate a tree with thousands of entities.

I will do some tests with a 45k entities instance this afternoon

[cconard96]

After testing with 40,000-45,000 entities we saw the selector generation take 30-60s after the sons cache was warmed up. This test had the entities directly under the root entity rather than in more of a tree hierarchy. I added more profiling and found that nearly all of the time taken was from the constructTreeFromList recursive calls.

The first optimization I tried can still be seen in the commit and it simply separated the single for-loop into two so only the unused data would be passed to the subsequent calls. For me, that optimization reduced the time taken from approximately a minute down to just over 1 second. However, this optimization only works well if the bulk of the items are in the first few levels. After I moved the 40k new entities under a child entity, then moved 2 groups of 10k down another level, the AJAX call jumped to 38 seconds.

The second optimization I tried pre-grouped the items by the parent ID before the first call to constructTreeFromList. This reduced the total AJAX call time from 38 seconds to 5 seconds. This second optimization seems like the best solution regardless of the complexity of the entity tree.

Although I only see constructTreeFromList used by getTreeForItem, I kept support for non-grouped items in case some plugin uses it.

[cedric-anne]

I did not find any occurence to getTreeForItem() or constructTreeFromList() in any plugin. Their only usage seems to be for the entity selector.

As the logic does not seems to have to be reusable, it could probably be optimized by combinating all of this into a single loop, instead of having:

• a loop on an iterator to generate an array,
• a loop on the array to generate a tree,
• a loop on the tree to format the output,
• a loop on the tree to add flags on expanded/selected element.

All of this could probably be done in Entity::getEntitySelectorTree(), and existing methods could be preserved and deprecated in GLPI 10.1, in case they are used by a plugin.

[cconard96]

The full AJAX request time is now down to 650ms for me following my last commit. The entity selector generation itself is 450ms. This is still with the 40,000 entities.

[cedric-anne]

Changes made in the DbUtils class can probably be reverted now.

Also, adding a test on Entity::getEntityTree() to validate its output would help to prevent regressions.

[cconard96]

Changes made in the DbUtils class can probably be reverted now.

Done

Also, adding a test on Entity::getEntityTree() to validate its output would help to prevent regressions.

That function is private. There is a test on the implementation of it instead Entity::getEntitySelectorTree.