magefan / module-rocketjavascript

https://magefan.com/rocket-javascript-deferred-javascript
Other
40 stars 8 forks source link

Update ResultPlugin.php #2

Closed pczerkas closed 5 years ago

pczerkas commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@pczerkas , we made a little bit another fix https://github.com/magefan/module-rocketjavascript/commit/9683a0dac3eb2d5eb62581d025c81b59595b6344

  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)
pczerkas commented 5 years ago

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.

  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)

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 commented 5 years ago

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