Importing styles is very inefficient

Kirby

Well-known member
Affected version
2.2.10
When importing a style to overwrite an existing style, the style import service calls \XF\Repository\Style::triggerStyleDataRebuild
PHP:
public function importFromXml(\SimpleXMLElement $document)
{
    $db = $this->db();
    $db->beginTransaction();

    $addOnId = (string)$document['addon_id'];

    $style = $this->getTargetStyle($document);

    $this->importAssets($style, $document->assets, $addOnId);
    $this->importPropertyGroups($style, $document->properties, $addOnId);
    $this->importProperties($style, $document->properties, $addOnId);
    $this->importTemplates($style, $document->templates, $addOnId);

    /** @var \XF\Repository\Style $styleRepo */
    $styleRepo = $this->repository('XF:Style');
    $styleRepo->triggerStyleDataRebuild();

    $db->commit();

    return $style;
}

This method then enqueues job \XF\Job\TemplateReuild
PHP:
public function triggerStyleDataRebuild()
{
    // this task is a subset of what we're doing so don't bother
    \XF::dequeueRunOnce('stylePartialRebuild');

    $this->app()->service('XF:Style\Rebuild')->rebuildFullParentList();

    $this->app()->jobManager()->enqueueUnique('styleRebuild', 'XF:Atomic', [
        'execute' => ['XF:TemplateRebuild', 'XF:StyleAssetRebuild', 'XF:StylePropertyRebuild']
    ]);
}

The job finally queries and recompiles all templates
PHP:
if ($this->data['skipCore'])
{
    $skipCoreSql = "AND (addon_id <> 'XF' OR style_id > 0)";
}
else
{
    $skipCoreSql = '';
}

$templateIds = $db->fetchAllColumn($db->limit(
    "
        SELECT template_id
        FROM xf_template
        WHERE template_id > ?
            {$skipCoreSql}
        ORDER BY template_id
    ", $this->data['batch']
), $this->data['templateId']);
if (!$templateIds)
{
    return $this->complete();
}

/** @var \XF\Service\Template\Compile $compileService */
$compileService = $app->service('XF:Template\Compile');

$done = 0;

foreach ($templateIds AS $templateId)
{
    $this->data['templateId'] = $templateId;

    /** @var \XF\Entity\Template $template */
    $template = $em->find('XF:Template', $templateId);
    if (!$template)
    {
        continue;
    }

    $template->getBehavior('XF:DevOutputWritable')->setOption('write_dev_output', false);

    $needsSave = $template->reparseTemplate(true);
    if ($needsSave)
    {
        // this will recompile
        $template->save();
    }
    else
    {
        $compileService->recompile($template);

This seems like an insane overhead as only templates that are (or were prior to the import) customized in the style; templates that are customized in a parent or an entirely different tree wouldn't be affected at all.

On one of our installs (that has 7 styles and 4 languages) this causes recompilation of ~ 40.000 templates while the style only has a ~ 10 customized templates.
 
Standard Code
Code:
time php cmd.php xf:style-archive-import --force --target=overwrite --overwrite-style-id=3 /tmp/test-style.zip > /dev/null

real    0m49,673s
user    0m34,656s
sys     0m5,464s

With a few class extensions to fix this (eg. only compile templates that actually have been changed)
Code:
time php cmd.php xf:style-archive-import --force --target=overwrite --overwrite-style-id=3 /tmp/test-style.zip > /dev/null

real    0m1,897s
user    0m0,644s
sys     0m0,160s
 
It's been a while since I specifically tried this to know for sure, but this change could be problematic if you have an existing template that includes another template that doesn't exist, and the missing template is added by the import. But worst case is that you would have to manually edit the other template to trigger a rebuild and get the inclusion to work. If it is problematic, your change could still be implemented as long as XenForo tracks wanted-templates and adds the wanting-template to the list of templates to be recompiled.
 
Included templates aren't inline at compile time, but instead are just a function call to render the included template.

Honestly XF should do this with all ungrouped phrases, this would collapse the number of templates by decoupling language<=>template relationship from what is a quadratic relationship to a linear one.
 
Top Bottom