Domain Config loading is resource intensive

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

I'm encountering the same problem. Example: loading /admin/structure/menu/manage/admin takes 45.8 (!) seconds. With 3232343-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.

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.

By using PathValidatorInterface::getUrlIfValidWithoutAccessCheck() instead of Url::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.

dieterholvoet → changed the visibility of the branch 2.0.x to hidden.

dieterholvoet → changed the visibility of the branch 3232343-performance-problem to hidden.

Using route information from $options, so without doing any route matching ourselves, I got it down to +-15.9s. These changes are on the 3232343-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.

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.

Update the patch to be working with the drupal core issue

Not sure what Drupal core issue you're referring to. Also, work is being done in a MR, so no need to post patches here.

Thanks for your hard work @dieterholvoet!

Hoping we will get a working solution sooner or later 🙏

Hi @DieterHolvoet, after applying your code changes, I get the following error: "The file "modules/contrib/domain/domain_source/domain_source.services.yml" does not contain valid YAML: Unexpected characters near "," at line 9 (near "- '@domain.negotiator',")."

It seems like the change in domain_source.services.yml contains a yml-formatting issue: while commas after yml list items seem to be valid yaml, my setup (using lando) seems to have issues parsing it. It could be fixed by either removing the commas or placing the list between square brackets and removing the hyphens (like the formatting was before).

@gillesv fixed!

Confirm that #24 https://www.drupal.org/project/domain/issues/3232343#comment-16031559 📌 Domain Config loading is resource intensive Needs work works great. With this patch module page loading dropped ~3.5s to 1.2s

Thank you all & https://www.drupal.org/u/seyfettinkahveci →

Hi @dieterholvoet,

While waiting for core to provide the route_name and route_parameters in the processOutbound method options parameter, I worked on an in-house lightweight route matcher that extract those values from the path.

You can see it implemented in MR 177 for issue 3271978 → .

Could it be a temporary fix until core issue is fixed? What do you think?

I did some profiling. I loaded the /admin/structure/menu/manage/admin page under different conditions:

• no patches: 49.3s
• patches from 🐛 Outbound path processors miss the route name Needs work & 📌 Domain Config loading is resource intensive Needs work : 19.6s
• patch from 🐛 DomainSource Outbound Path Processor endless Loop Closed: cannot reproduce : 26.8s

Conclusion: good improvement, but not as good as the solution with the core patch, so I would leave this issue open for the time being. The new solution introduces a lot of custom code, but if you're willing to maintain all that I'd say go for it!

Fixed by issue 🐛 DomainSource Outbound Path Processor endless Loop Closed: cannot reproduce .

This will approximately cut execution time by half.

For even better performance, the following Core patch has to be applied: 🐛 Outbound path processors miss the route name Needs work .