- First commit to issue fork.
- @dtfabio opened merge request.
- π§πͺBelgium dtfabio Ninove
Hi everyone,
We are using the patch β3232343-2-domain_source_performance_fix.patchβ in production currently and so I tried to re-roll the patch from seyfettinkahveci to the 2.0.x version of the module and created an MR for this.
Apparently, something did go wrong which meant that not all the modifications from the 2.0.x were present. I tried to fix this, but noticed that something is still not correct.
I will adjust this and pay better attention to prevent this kind of thing in the future.
Greetings,
Fabio
- Issue was unassigned.
- π§πͺBelgium dieterholvoet Brussels
I'm encountering the same problem. Example: loading
/admin/structure/menu/manage/admin
takes 45.8 (!) seconds. With3232343-2-domain_source_performance_fix.patch
applied, it only takes 15.3 seconds. I understand that certain code is being disabled that shouldn't be, but a performance impact like this is unacceptable. It impacts every page of your website and especially the ones where a lot of urls are being generated. I'll try to look into this more. - π§πͺBelgium dieterholvoet Brussels
The problem is that this module is inherently slow and there isn't much we can do about it. For every generated url, we have to check if there's an associated entity and if there is, we have to check its domain access field to know if we need to rewrite the domain. There isn't really a way around that, that's the whole point of the module.
According to my profiler, the most expensive calls are
Url::fromUserInput()
and$this->aliasManager->getPathByAlias()
, they account for 80% of the time spent in the path processor. I'll check if these have cheaper alternatives. - π§πͺBelgium dieterholvoet Brussels
By using
PathValidatorInterface::getUrlIfValidWithoutAccessCheck()
instead ofUrl::fromUserInput()
and$this->aliasManager->getPathByAlias()
, I'm able to reduce the time from +-46sec to +-34sec. Still slow, but it's something. I'll start a new MR with this change.In an ideal world, we wouldn't have to do any route matching at all. $options['route'] already contains the matched route, the problem is that we don't know the route name, which is something that's being proposed in π Outbound path processors miss the route name Needs work . I'll also create a MR that builds on top of the change in that core issue.
- π§πͺBelgium dieterholvoet Brussels
dieterholvoet β changed the visibility of the branch 2.0.x to hidden.
- @dieterholvoet opened merge request.
- π§πͺBelgium dieterholvoet Brussels
dieterholvoet β changed the visibility of the branch 3232343-performance-problem to hidden.
- π§πͺBelgium dieterholvoet Brussels
Using route information from
$options
, so without doing any route matching ourselves, I got it down to +-15.9s. These changes are on the3232343-use-info-from-options
branch, but they do depend on a patch from π Outbound path processors miss the route name Needs work . That seems like the way to go, but we'll have to work on getting that core change in first. - π§πͺBelgium dieterholvoet Brussels
The issue description is about two separate issues. I'll narrow this down to the
DomainSourcePathProcessor
issue, since @agentrickard already gave feedback on the(!$this->storage->listAll('domain.config.')
check. - @dieterholvoet opened merge request.