FlowQuery add limit to find operation to improve performance (and dont slice in php)

[bwaidelich]

//searchAction.targetNode = ${q(site).find('[instanceof Neos.DocsNeosIo:Document.SearchResultsPage]').get(0)} is currently slow because it loads ALL nodes in a CTE; and then finds the FIRST SearchResultsPage.

=> .find() could be more clever:

• if the desired node type is a Document node, we know we must only traverse Document nodes.
• => we could add an additional "traversalNodeTypeFilter" into FindDescendantNodesFilter and use this for the traversal step.

[bwaidelich]

(this comes from @skurfuerst originally, I just turned it into an issue)

Making FlowQuery operations even more clever makes me concerned because they already contain a lot of domain logic.
I would suggest to go for one of these alternative routes instead:

a) defer fetching of nodes to the final operation

That would mean, that common node operations (filter, children, find, ...) would not actually load the nodes, but put the filter options into the context¹ and get() would actually invoke the filter.

drawbacks / issues

1. This can be breaking because of other operations of Fusion code expecting the result object in the context
2. For some cases we'd still need to load the intermediate result (e.g. $q(node).children('foo').children('bar')})

b) introduce new operations that use the new Subgraph API

For each of the subgraph methods (findChildNodes, findDescendantNodes, ...) there could be a corresponding operation, moving most of the heavy shifting to the database layer

drawbacks / issues

1. having findDescendantNodes and find etc. might be confusing
2. how to translate the PHP method to easy-to-use Operations? the filter API might be to low-level for Fusion

e.g.

$q(node).findDescendantNodes({nodeTypeConstraints: 'Neos.DocsNeosIo:Document.SearchResultsPage', limit: 1})

or sth like

$q(node).findDescendantNodes().filterNodeTypes('Neos.DocsNeosIo:Document.SearchResultsPage').get(0)

?

---

¹ this could be the filter object itself or some lazy result/query object, e.g. FindDescendantNodesQuery:

<?php
namespace Neos\ContentRepository\NodeAccess\SubgraphQueries;

final class FindDescendantNodesQuery {
    public function __construct(
        public readonly NodeAggregateId $entryNodeAggregateId,
        public readonly FindDescendantNodesFilter $filter,
    );
    
    public function withFilter(FindDescendantNodesFilter $filter): self
    {
      return new self($this->entryNodeAggregateId, $filter);
    }
}

[bwaidelich]

Update: I think I completely misunterstood the issue and I somehow skipped this part:

we could add an additional "traversalNodeTypeFilter" into FindDescendantNodesFilter and use this for the traversal step.

While I think, this is a very useful performance optimization, it is IMO too low-level for the public Subgraph API. But maybe we can do this automatically in the findDescendantNodes code.
The DocumentUriPath-projection also relies on Document node types so it might be OK to do this here as well? (not relevant, because we only need it at read time for the Subgraph and already inject the NodeTypeManager)

Maybe we can extend NodeTypeConstraintsWithSubNodeTypes by some method that allows to determine whether only document or only content nodes are affected?

[mhsdesign]

Hi im not sure i understand correctly, but isnt this performance optimization already implemented in "find"?

neos-development-collection/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php

Lines 263 to 278 in 2b4e5b6

	protected function addNodesByType(NodeTypeName $nodeTypeName, array $entryPoints, array $result, ContentRepository $contentRepository): array
	{
	$nodeTypeFilter = NodeTypeConstraints::create(NodeTypeNames::with($nodeTypeName), NodeTypeNames::createEmpty());
	foreach ($entryPoints as $entryPoint) {
	/** @var ContentSubgraphInterface $subgraph */
	$subgraph = $entryPoint['subgraph'];
	/** @var Node $node */
	foreach ($entryPoint['nodes'] as $node) {
	foreach ($subgraph->findDescendantNodes($node->nodeAggregateId, FindDescendantNodesFilter::create(nodeTypeConstraints: $nodeTypeFilter)) as $descendant) {
	$result[] = $descendant;
	}
	}
	}
	return $result;

When there is only an instance of filters we translate the call directly to only fetch nodes of type x.

---

edit i misunderstood - this is about the never ending story of having no "limit" option that affects the sql query in flow, but that we only slice in php - which is slower.

first of all the findDescendantNodes would need a limit filter itself ;)

[bwaidelich]

first of all the findDescendantNodes would need a limit filter itself ;)

The FindDescendantNodesFilter (and FindBackReferencesFilter, FindChildNodesFilter, FindPrecedingSiblingNodesFilter, FindReferencesFilter and FindSucceedingSiblingNodesFilter) already have a pagination criterion that can be used to limit the results.

But this issue is about the internal behavior of the ContentSubgraph that loads all nodes in a CTE

[mhsdesign]

related (about performance) #4600

[mhsdesign]

As exactly this topic just fell on a beta user knee (slack) i did some investigation.

Now this is a very likely node structure

- site
    - blogOverview (Vendor:Document.BlogOverview)
        - blogPage 1 (Vendor:Document.BlogPage)
        - blogPage 2 (Vendor:Document.BlogPage)
        - blogPage 3 (Vendor:Document.BlogPage)
        - ...
    ...

And in Neos 8 is was common practice to get the overview page via find

${q(site).find('[instanceof Vendor:Document.BlogOverview]').get(0)}

Or also the first blog entry

${q(site).find('[instanceof Vendor:Document.BlogPage]').get(0)}

This is in Neos 9 tremendously slow. We advised in slack to use deterministic node aggregate ids (which is currently not really known for the end users) or to make the blog page tethered. Because fetching a named child node or the first child node gives a huge speedup:

$q(site).find('#blogOverview').children('[instanceof Vendor:Document.BlogPage]').get(0)}

Now similar to "b) introduce new operations that use the new Subgraph API" we discussed for several reasons to introduce new flow query operations, mainly because they are currently bloated and need to split up and because we wanted also to allow to use limit=1 in the actual sql queries rather than fetching all nodes in memory and then discarding all except one via get(0): #5434

As outlined here i tested the flow-queries and using the subgraphs native pagination limiting to 1:

${q(site).findByCriteria('Vendor:Document.BlogPage', null, {limit:1}).get(0)}

But my preliminary results were not promising. In fact there was no real difference noticeable. And that makes sense because the limiting only optimises only one thing so far: how many nodes are returned via sql. Now the node objects are all slim and there is little data to transfer so its not noticeable finding one or twenty blog pages and then discarding results.

The real performance hit seems to be how we execute the sql query and that there is no optimisation for the limit 1 case.

The current implementation of findDescendantNodes uses a CTE to build the whole node tree from the entry node with all joined nodes in the specified dimension and content stream. Only we dont return ALL the nodes now but select from the CTE only what we wanted to filter and also limit it then:

neos-development-collection/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentSubgraph.php

Lines 622 to 631 in f20b28c

	$queryBuilderCte = $this->nodeQueryBuilder->buildBasicNodesCteQuery($entryNodeAggregateId, $this->contentStreamId, $this->dimensionSpacePoint, 'tree', 'n');
	if ($filter->nodeTypes !== null) {
	$this->nodeQueryBuilder->addNodeTypeCriteria($queryBuilderCte, ExpandedNodeTypeCriteria::create($filter->nodeTypes, $this->nodeTypeManager));
	}
	if ($filter->searchTerm !== null) {
	$this->nodeQueryBuilder->addSearchTermConstraints($queryBuilderCte, $filter->searchTerm);
	}
	if ($filter->propertyValue !== null) {
	$this->nodeQueryBuilder->addPropertyValueConstraints($queryBuilderCte, $filter->propertyValue);
	}

The thesis is that there is no optimisation potential even for the limit=1 case in the database because the full tree was read in the database either way we just didnt return it fully.

Now we discussed if we could leverage some kind of termination condition during the execution of the CTE to filter the nodes while we are descending and once there is a node found and its limit=1 we will simply not descend further.

I attempted to implement a rough draft in #5436 which technically seems to work, but its benchmarks are not that promising so far/

Benchmark

The following results where tested on a project with X nodes in a similar structure like above.

Type	Entry	Filter	Time (in sec) of 5x	Query Cost
Descendants Limit 1 (9.0)	site	[Vendor:Document.BlogPage]	0.080411863327026	0.117153
Descendants Limit 1 (Termination)	site	[Vendor:Document.BlogPage]	0.070744180679321	0.117153
Descendants Limit 1 (9.0)	blogOverview	[Vendor:Document.BlogPage]	0.012453269958496	0.117153
Descendants Limit 1 (Termination)	blogOverview	[Vendor:Document.BlogPage]	0.0026803970336914	0.117153
Descendants Limit 1 (9.0)	site	[Vendor:Document.BlogOverview]	0.078737640380859	0.117153
Descendants Limit 1 (Termination)	site	[Vendor:Document.BlogOverview]	0.069754028320312	0.117153
Children Limit 1 (9.0)	blogOverview	[Vendor:Document.BlogPage]	0.00080294609069824	0.014480

It seems that using the findChildNodes query is just unbeaten. There seems to be a little performance gain for some cases with using the termination but not for the problematic "site" entry case.

Further things we might need to benchmark and discuss:

• bench and introduce a maxDepth option to avoid traversing all levels in depth - each getting more expensive
• bench and introduce a traversalNodeTypeFilter contraire to FlowQuery add limit to find operation to improve performance (and dont slice in php) #4205 (comment) so we limit the amount of nodes we need to follow / descend into