Skip to content

Update ResultPlugin.php #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

pczerkas
Copy link

@pczerkas pczerkas commented Oct 8, 2018

Hello,

This pull request fixes problem, when scripts in source were one next to the other and tries to optimize string replacement step.

BR,
Przemek

Hello,

This pull request fixes problem, when scripts in source were one next to the other and tries to optimize string replacement step.

BR,
Przemek
@magefan
Copy link
Owner

magefan commented Oct 8, 2018

Thank you @pczerkas,

Could you please send to us some example of HTML so we can test this before the push to master?

Regards,
Magefan team

@magefan
Copy link
Owner

magefan commented Oct 9, 2018

@pczerkas , we made a little bit another fix
9683a0d

  1. We need to $start++ when attribute data-rocketjavascript="false" is in use, otherwise can get infinite loop.
  2. We didn't add substr_replace as with strpos it works much longer (more than 5 times longer) than just str_replace (tested on PHP 7.0.32)
@magefan magefan closed this Oct 9, 2018
@pczerkas
Copy link
Author

Hello @magefan team

@pczerkas , we made a little bit another fix
9683a0d

  1. We need to $start++ when attribute data-rocketjavascript="false" is in use, otherwise can get infinite loop.
  1. We didn't add substr_replace as with strpos it works much longer (more than 5 times longer) than just str_replace (tested on PHP 7.0.32)

Hmm, that's strange - today i've done some timings and it looks like it was micro-optimisation, i.e. it didn't helped more than 5% in execution time.

But I have another version for you to test, this time it speedups things x2 on my side ;) It leverages the fact that stripos() can be further optimized with strpos() (which is much faster), as it has constant prefixes "<" and "</" in needle.

Timings should display on page top.

public function aroundRenderResult(
    \Magento\Framework\Controller\ResultInterface $subject,
    \Closure $proceed,
    ResponseHttp $response
) {
    $result = $proceed($response);
    if (PHP_SAPI === 'cli' || $this->request->isXmlHttpRequest() || !$this->isEnabled()) {
        return $result;
    }

    $html = $response->getBody();

    $stripos_prefix = function(
        string &$haystack,
        string $prefix,
        string $needle,
        int $offset = 0
    ) {
        $prefixStrlen = strlen($prefix);
        $needleStrlen = strlen($needle);
		
        $needle = strtolower($needle);

        while (false !== ($offset = strpos($haystack, $prefix, $offset))) {
            if ($needle === strtolower(substr($haystack, $offset + $prefixStrlen, $needleStrlen))) {
                return $offset;
            }

            $offset += $prefixStrlen + $needleStrlen;
        }

        return false;
    };

    $startTagPrefix = '<';
    $startTag = 'script';

    $endTagPrefix = '</';
    $endTag = 'script>';

    $endTagStrlen = strlen($endTagPrefix . $endTag);

    $timeStart = microtime(true);

    $scripts = [];
    $start = 0;
    $i = 0;
    while (false !== ($start = $stripos_prefix($html, $startTagPrefix, $startTag, $start))) {
        if ($i++ > 1000) {
            return $result;
        }

        if (false === ($end = $stripos_prefix($html, $endTagPrefix, $endTag, $start))) {
            break;
        }

        $len = $end + $endTagStrlen - $start;
        $script = substr($html, $start, $len);

        if (false !== stripos($script, self::EXCLUDE_FLAG_PATTERN)) {
            continue;
        }

        $html = substr_replace($html, '', $start, $len);

        $scripts[] = $script;

        $end = $start;
    }

    $scripts = implode(PHP_EOL, $scripts);
    if ($end = $stripos_prefix($html, '</', 'body>', $end)) {
        $html = substr_replace($html, $scripts, $end, 0);
    } else {
        $html .= $scripts;
    }

    echo(microtime(true) - $timeStart);

    $response->setBody($html);

    return $result;
}

`

@magefan
Copy link
Owner

magefan commented Oct 11, 2018

@pczerkas , thank you for the suggestion, we will check this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants