Optimize inline link destination parsing (#1141)

Author: colinodell

CHANGELOG.md

@@ -9,6 +9,8 @@ Updates should follow the [Keep a CHANGELOG](https://keepachangelog.com/) princi
 ### Changed
 - Improved performance of reading single characters from multibyte lines
 - Improved performance of locating the next non-space character on lines without tabs
+- Optimized `Cursor::advanceToNextNonSpaceOrNewline()` to scan the line in place instead of copying everything left in the block on every call
+- Optimized inline link destination parsing to scan the line in place, so its cost follows the length of the destination rather than the length of everything left in the block
 
 ### Fixed
 - Fixed heading permalinks rendered with `aria-hidden="true"` remaining in the keyboard tab order; they are now also given `tabindex="-1"`, as a focusable element removed from the accessibility tree has no accessible name to announce when focused (WCAG 4.1.2)

src/Parser/Cursor.php

@@ -469,13 +469,22 @@ public function advanceToNextNonSpaceOrNewline(): int
             return 0;
         }
 
-        $matches = [];
-        \preg_match('/^ *(?:\n *)?/', $this->getRemainder(), $matches, \PREG_OFFSET_CAPTURE);
+        // A partially-consumed tab leaves the cursor sitting on the tab itself, which the check
+        // above has already returned on, so only real spaces and newlines reach this point and no
+        // tab expansion is needed.
+        //
+        // Spaces and newlines are single-byte ASCII characters which can never appear inside a
+        // multibyte UTF-8 sequence, so the run is measured at the byte level and each byte
+        // consumed is exactly one character. Scanning the line in place keeps the cost of each
+        // call proportional to the run it consumes, rather than to the length of everything left
+        // in the block, which is what building the remainder first charged for.
+        $byteOffset = $this->isMultibyte ? $this->byteOffset($this->currentPosition) : $this->currentPosition;
 
-        // [0][0] contains the matched text
-        // [0][1] contains the index of that match
-        \assert(isset($matches[0]));
-        $increment = $matches[0][1] + \strlen($matches[0][0]);
+        $increment = \strspn($this->line, ' ', $byteOffset);
+        if (($this->line[$byteOffset + $increment] ?? '') === "\n") {
+            $increment++;
+            $increment += \strspn($this->line, ' ', $byteOffset + $increment);
+        }
 
         $this->advanceBy($increment);
 

src/Util/LinkParserHelper.php

@@ -93,12 +93,19 @@ public static function parsePartialLinkTitle(Cursor $cursor, string $endDelimite
 
     private static function manuallyParseLinkDestination(Cursor $cursor): ?string
     {
-        $remainder  = $cursor->getRemainder();
+        // The destination always ends at the first whitespace or unbalanced ")", so scan the line
+        // in place from the cursor rather than materializing the remainder: the cost of finding it
+        // should follow the length of the destination, not the length of everything left in the
+        // block. A partially-consumed tab needs no special handling - getRemainder() would expand
+        // it into leading spaces and the scan would stop on the first one, exactly as it stops on
+        // the tab itself here.
+        $line       = $cursor->getLine();
+        $start      = $cursor->getBytePosition();
         $openParens = 0;
-        $len        = \strlen($remainder);
+        $len        = \strlen($line) - $start;
         for ($i = 0; $i < $len; $i++) {
-            $c = $remainder[$i];
-            if ($c === '\\' && $i + 1 < $len && RegexHelper::isEscapable($remainder[$i + 1])) {
+            $c = $line[$start + $i];
+            if ($c === '\\' && $i + 1 < $len && RegexHelper::isEscapable($line[$start + $i + 1])) {
                 $i++;
             } elseif ($c === '(') {
                 $openParens++;
@@ -125,7 +132,7 @@ private static function manuallyParseLinkDestination(Cursor $cursor): ?string
             return null;
         }
 
-        $destination = \substr($remainder, 0, $i);
+        $destination = \substr($line, $start, $i);
         $cursor->advanceBy(\mb_strlen($destination, 'UTF-8'));
 
         return $destination;

tests/pathological/test.php

@@ -188,6 +188,15 @@
         'input' => static fn($n) => \str_repeat('[a](b', $n),
         'expected' => static fn($n) => '<p>' . \str_repeat('[a](b', $n) . '</p>',
     ],
+    'Unclosed inline links with whitespace after the paren' => [
+        // The space is load-bearing: it routes each "(" through the whitespace skip in
+        // Cursor::advanceToNextNonSpaceOrNewline() as well as the destination scan in
+        // LinkParserHelper. Every other case here puts a non-space directly after the "(", so the
+        // destination scan is the only one they cover.
+        'sizes' => [10_000, 50_000, 200_000],
+        'input' => static fn($n) => \str_repeat('[a]( b', $n),
+        'expected' => static fn($n) => '<p>' . \str_repeat('[a]( b', $n) . '</p>',
+    ],
     'Unclosed inline links (3)' => [
         'ref' => 'https://github.com/commonmark/commonmark.js/issues/129',
         'sizes' => [1_000, 10_000, 100_000],