Repository navigation
Conversation
Escape::html ran nine sequential str_replace passes over the full rendered output. The escape keys and token keys are disjoint and strtr never rescans replaced text, so one strtr pass gives identical output at about a third of the cost.
Twig tags and GDScript ran one preg_match_all per keyword (38 and 36 patterns). Every keyword is a whole word bounded by \b, so a single alternation yields exactly the same tokens with one regex pass.
…guages The pattern and injection caches were keyed by spl_object_id. Injections created a new language object for every match, so the caches missed on each nested parse, and a freed object's id could be reused by a different language, which then got the stale patterns. Store the caches in a WeakMap keyed by the language instance, and let the four injections that built a language per match reuse one instance.
…ting Patterns used (.|\n)*? to match across lines. PCRE handles the alternation group with one backtracking frame per character, so a block of roughly 50KB or more exhausts the JIT stack. preg_match_all then returns false and the block is silently left unhighlighted. [\s\S] matches the same characters as (.|\n) without the group, so the output is unchanged and the size limit is gone.
…ctions Lets a pattern or injection declare substrings it needs, so the highlighter can skip it when the content contains none of them.
…-8 errors
Unclosed {- and similar markers made the marker injections rescan to the
end of the content for every opener. 100KB of them took ~650ms. Matching
now stops after the last valid closer, and Addition/Deletion build their
output in one pass instead of one substr_replace per marker.
A reused withGutter() highlighter kept line marks from the previous
parse, and content with invalid UTF-8 made the gutter throw a TypeError
because preg_split returned false. Marks are now cleared per parse, and
a byte-level split with the same line breaks is used as a fallback.
The Base markers also declare the substrings they need, so the
highlighter can skip them.
…bugs Lazy (\\"|.)*? string patterns and (.)* comment patterns exhausted the PCRE JIT stack on a single line of ~6KB, after which every later match of the same pattern was silently lost. The rewrites match the same text without per-character backtracking frames. Also: - PHP keywords: (?<!\$) in a double-quoted PHP string compiled to the end anchor (?<!$), so $new in function parameters got a keyword span. - Python: a missing | meant 'and' and '_' were never keywords. - StaticClassCallPattern was quadratic on long identifiers. - RubyLexer kept the last input in a static property for the life of the process; it is now held per language instance. - Comment patterns stop at an unclosed opener with (*COMMIT) instead of rescanning to the end (quadratic without JIT).
…mory - Built-in patterns that use IsPattern::match() get their final regex and token type resolved once per language instead of on every parse. - Patterns and injections marked with #[RequiresSubstring] are skipped when the content contains none of the substrings. - Tokens from one pattern are cloned from a prototype, and render and grouping loops do less work per token. - Tokens are sorted on one packed integer key instead of array_multisort over four arrays. A 2MB PHP file no longer runs out of memory at 128M. - RenderTokens only caches markup for enum token types, keyed by class and case. Custom class names grew the cache without bound, and a custom TokenType returning a built-in value got the wrong markup. Adds EdgeCaseBench for the pathological inputs.
Injections declare the literal every match needs, so the highlighter skips them on content that cannot match. PhpGenericTypeInjection now uses one constant pattern instead of building a regex per type name, which also stops type names from corrupting earlier markup. Injection regexes stop at an unclosed opener with (*COMMIT).
ArtMin96
changed the title
Benchmark ResultsComparison of Open to see the benchmark results
Generated by phpbench against commit 6be53a0 |
Is this a requirement for all injections or an opt-in performance boost? |
How the engine treats it (
|
|
@brendt What it does. Take Why it's safe. Every regex where I used it has a closer that doesn't depend on the opener ( Measured on 20,000 unclosed Match results were identical in every case. With JIT on, the speed gain is small: PCRE's JIT already handles this input well. The verb protects servers where JIT is off. Some hosts disable it, and PHP falls back to the interpreter when JIT can't allocate memory. On those servers a single unclosed comment in pasted code can take seconds to render. |
xHeaven
left a comment
xHeaven
left a comment
There was a problem hiding this comment.
Note
This message is generated by gpt-6.1-sol on behalf of xHeaven. It may contain errors—verify important details.
Requesting changes on ebf9682c7dd34ae769f754afee41c768ee6218ee (compared with 3d96482ee77208c8e445d4f68667918c7f440f62).
The performance improvements hold up in local testing: all 35 fixtures improved, with a 30.4% reduction in their summed execution times. The full suite passed (1,197 tests / 1,712 assertions), as did PHPStan and the style check. The additional review covered all 114 changed files and included focused tests, differential probes, lifecycle checks, and CommonMark integration.
Four independently reproduced regressions remain, detailed inline:
- P2: packed sort keys lose precision under
SORT_NUMERIC, changing token precedence on very large inputs. - P2: unterminated GraphQL strings containing escaped quotes trigger quadratic processing.
- P3: compiled regexes and token types become stale when mutable built-in or certain custom patterns are reconfigured.
- P3: ordinary Python
_identifiers before a colon receive wildcard keyword styling.
Please also add dedicated behavioral tests for CombinedGdscriptKeywordPattern and CombinedTwigKeywordPattern. Both require constructor arguments, so the attribute-based pattern test discovery does not exercise them. Include multiple matches, near misses, identifier boundaries, and capture offsets.
Local verification used PHP 8.5.12. PHP 8.4 passed hosted CI, but was not rerun locally. The generated and differential coverage is bounded; the inline comments state the conditions for each failure.
|
@ArtMin96 are you ok looking at Mark's feedback, or want me to take a look? |
|
@brendt I’ll check them all and apply the fixes that are valid. I’ll do it tomorrow. |
|
Thanks! |
Compiled regexes were reused for any IsPattern class under the Tempest\Highlight namespace. KeywordPattern::caseInsensitive() and custom classes in that namespace could change their pattern between parses, and the old regex kept being used. Freeze only classes whose file lives in this package's src directory and whose properties are all readonly. OperatorPattern and TwigKeywordPattern never change after construction, so their properties are now readonly and they keep the fast path.
SORT_NUMERIC compares integers as doubles. Packed keys above 2^53, reached from inputs of about 95MB, could compare equal, so a shorter token stayed ahead of a longer one starting at the same offset. The default flag compares two integers exactly.
An unclosed string with escaped quotes took quadratic time: after the string failed, matching restarted at every escaped quote inside it and scanned the same text again. A 262KB input took about 2 seconds. Restarts inside the scanned body always fail the same way, so (*SKIP) resumes after it. Fuzzing 200k inputs against the previous regex found no difference in matches.
Restoring the missing alternation also enabled the _ branch wherever a colon follows, so parameters, annotated names, dict keys, lambdas and slices styled _ as a keyword. Require a preceding "case" so only the wildcard in "case _:" matches.
Both patterns take their keyword list in the constructor, so the PatternTest attribute discovery cannot build them. Cover multiple matches with offsets, word boundaries, case, duplicates, the empty list, and equality with one pattern per keyword on the bench fixture.
xHeaven
left a comment
xHeaven
left a comment
There was a problem hiding this comment.
Note
This message is generated by gpt-6.1-sol on behalf of xHeaven. It may contain errors—verify important details.
Requesting changes on 98ce35e302af2eaa60240288dc2dc67fde9a84d1.
The packed-key precision, GraphQL rescanning, and mutable-pattern caching findings are fixed, and the combined-keyword behavioral tests have been added. The Python underscore fix remains partial: the two inline comments cover a new multiple-whitespace omission and a remaining false positive after a comment ending in case.
Reviewed all 13 files changed since ebf9682, including independent standards and spec passes. Local verification passed: 1,228 repository tests / 1,749 assertions, PHPStan, and the style check. Both additional public-interface Python regression probes fail as described inline. Hosted checks, including PHP 8.4 and the performance check, are green. Local runs used PHP 8.5.12; generated probes are bounded.
The `(?<=\bcase\s)_` lookbehind missed `case _:` and `case \t_:`, took its context from a comment ending in "case", and ignored wildcards in or, sequence, star, class and mapping patterns. PyCaseWildcardPattern scans `case` clauses whose enclosing block is a `match` statement (or the top level of a snippet) and marks `_` names in the pattern, skipping strings, comments, guards and bodies. Its results match the wildcards Python's ast reports across 2,467 generated programs.
|
Pushed. While I was on the
|
xHeaven
left a comment
xHeaven
left a comment
There was a problem hiding this comment.
Note
This message is generated by an AI agent on behalf of @xHeaven. It may contain errors—verify important details.
Follow-up on 6be53a001e8b33ec9f0d74efc19879705b13f10e, compared with 98ce35e.
The two previous Python findings are fixed: multiple whitespace characters after case now work, and a comment ending in case no longer turns an ordinary annotation into a wildcard. The separate scanner also adds guarded and nested-pattern coverage.
Standards
All four changed files were reviewed independently. No separate design-smell finding. The identifier-boundary and multiline requirements in AGENTS.md remain incomplete in the two scanner paths described inline.
Spec
The claim that _ is styled only where Python reads it as a wildcard still has three confirmed P3 exceptions: ordinary annotations are accepted as clauses, ordinary multiline expressions can erase match context, and continued attribute/field names are styled as wildcards. Details and valid Python reproductions are inline. Source text remains intact.
Verification: 745 focused repository tests / 813 assertions passed; scoped PHPStan and the four-file dry-run style check passed. Ninety additional valid Python cases matched AST wildcard offsets. Three additional public-interface regression tests fail as described inline; the ordinary-annotation test passes at the previous head. All hosted checks, including PHP 8.4 and the performance check, are green. The Python fixture benchmark completed successfully on both heads with similar timings.
Local verification used PHP 8.5.12. Full PHPUnit, full static analysis, and all-language benchmarks were not repeated for this four-file update; generated coverage is bounded. Standards: two boundary/multiline failures, no independent smell. Spec: three P3 findings.
| continue; | ||
| } | ||
|
|
||
| if ($char === ':' && $depth === 0) { |
There was a problem hiding this comment.
Note
This message is generated by an AI agent on behalf of @xHeaven. It may contain errors—verify important details.
[P3] Distinguish ordinary annotations from case clauses
The scanner accepts the annotation's colon as a case delimiter and marks an ordinary subscript identifier as a wildcard:
case = {}
_: int = 0
case[_]: int = 1Actual final line: case[<span class="hl-keyword">_</span>]: …. Expected: case[_]: …, with _ plain. This is a module-level annotated assignment, not a match clause. case(_).value: int = 1 also incorrectly highlights the call argument.
The dictionary example both parses and executes successfully in Python. A public Highlighter::parse() regression test passes at 98ce35e and fails here. Please distinguish these ordinary statements before accepting a top-level colon, and add rendered annotation negatives alongside the fragment tests.
|
|
||
| $indent = $this->indentWidth(substr($content, $lineStart, $indentLength)); | ||
|
|
||
| while ($blocks !== [] && $blocks[array_key_last($blocks)][0] >= $indent) { |
There was a problem hiding this comment.
Note
This message is generated by an AI agent on behalf of @xHeaven. It may contain errors—verify important details.
[P3] Preserve match context across ordinary expression continuations
Only continued headers and strings are excluded from indentation tracking. A continued ordinary expression can therefore pop the enclosing match block:
match x:
case 0:
y = (
0
)
case _:
passThis is valid Python: indentation inside parentheses does not change the surrounding block. Actual final clause: case _:. Expected: case <span class="hl-keyword">_</span>:, as at 98ce35e.
The unindented 0 is treated as a statement, removes the match frame, and leaves the later case under a non-match frame. Please track ordinary bracket/backslash continuations before changing the indentation stack, and add a rendered regression. Confirmed with Python ast.parse() and through the public highlighter.
|
|
||
| if ($word === 'if' && $depth === 0) { | ||
| $inPattern = false; | ||
| } elseif ($inPattern && $word === '_' && $previous !== '.' && ! preg_match('/\G[ \t\f]*(?:=(?!=)|\.)/', $content, $nameOrAttribute, 0, $next)) { |
There was a problem hiding this comment.
Note
This message is generated by an AI agent on behalf of @xHeaven. It may contain errors—verify important details.
[P3] Skip logical whitespace when distinguishing names from wildcards
The lookahead skips only horizontal whitespace before . or =, but valid patterns can continue those constructs across lines:
match x:
case {
_
.key: _
}:
passActual: both underscores receive hl-keyword. Expected: only the mapping value does; the first underscore is the name in the attribute expression _.key. The first underscore was correctly plain at 98ce35e.
The same issue affects a class-pattern field name split before =, for example case Point(_ followed by a newline and =value):. That underscore names a field and must remain plain. Please account for legal newlines, comments, and continuations when checking the next significant token. Both examples pass Python ast.parse() and reproduce through Highlighter::parse().
|
@ArtMin96 on second thought, I would like this work to be split into separate PRs. It's too much to be able to confidently review. |
Highlighting is about 32% faster across the 35 bench fixtures (Twig 63%, Blade 40%). Output is byte-identical except where it was wrong. Profiling turned up a number of bugs, most of them also on
main, and they are fixed here with a failing test each.Performance
Escape::htmlis onestrtrpassstr_replacepasses over the whole output. The keys don't overlap, so one pass gives the same result.preg_match_allper keyword (38 and 36). Whole-word keywords give the same tokens from one alternation."/{$pattern}/"andgetPattern()were rebuilt on every call, including nested parses. CustomPatternclasses still go throughmatch().#[RequiresSubstring]attributearray_multisortover four arrays was the peak memory point.Bug fixes
mainspl_object_id{-markersmemory_limit=128MFoo::after a 50K-char identifierfunction f($new)$<keyword>new</keyword>(lookbehind compiled to an end anchor)and,case _|)andis a keyword;_only where Python reads it as a wildcardwithGutter()highlighterTypeError{:class:...:}with many class namesRubyLexerPhpGenericTypeInjectionBenchmarks
phpbench with Xdebug off.
main, the first commit set and this branch ran alternately, and the faster of two rounds counts.mainEvery language got faster. On 50KB inputs Twig went from 15.0 to 4.8ms, Blade from 12.2 to 5.2ms and PHP from 7.1 to 4.8ms.
EdgeCaseBenchcovers the pathological inputs above.Behaviour changes
Gutter marks now reset after each parse. Marks added with
addIcon/addClassbefore callingparse()still apply to that parse. Ruby patterns accept an optional sharedRubyLexer, and the default stays as before.Verification
I snapshotted 8,388 outputs from
main: every fixture parsed as every language, three themes, gutter on and off, 30KB inputs and adversarial snippets. The only differences are the fixes above. Running the branch with skipping disabled gives identical output over that corpus and 180k fuzzed inputs. Every regex rewrite was fuzzed against the original with no differences, apart from the intendedPhpGenericTypeInjectionfix. There are 1,252 tests; phpstan and php-cs-fixer are clean, and each commit passes on its own.Left for later
Swift's recursive patterns and Markdown emphasis still fail on very long lines. Svelte and Vue highlight some content twice, Blade echo and multiple
<script>blocks match greedily, and Pythonf(a = 1)gets no argument token. Custom classes with many distinct names are still quadratic,PhpHeredocInjectionbuilds a regex per language name, and blocks of about 1MB hitpcre.backtrack_limit.