Skip to content

perf: Read Token::$id once per token in Lexer - #1171

Merged
nikic merged 1 commit into
nikic:masterfrom
samsonasik:perf-read-lexer
Sep 30, 2026
Merged

nikic merged 1 commit into
nikic:masterfrom
samsonasik:perf-read-lexer

Conversation

@samsonasik

Copy link
Copy Markdown
Contributor

Lexer::postprocessTokens() reads Token::$id twice for every token. Token extends the internal PhpToken class, so each property read is comparatively expensive.

This PR reads the token ID once and reuses it.

Tested faster:

Corpus Tokens scanned Lexer gain
PHP-Parser vendor 501,000 34.22 → 31.32 ms (~8.5%)
laravel-framework 22,898,098 1586.35 → 1365.66 ms (~13.9%)

@samsonasik

Copy link
Copy Markdown
Contributor Author

@staabm @nikic ready for merge 👍

@nikic
nikic merged commit 651e7a9 into nikic:master Sep 30, 2026
12 checks passed
@staabm

staabm commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Token extends the internal PhpToken class, so each property read is comparatively expensive.

thats a interessting fact, I did not know about

@samsonasik
samsonasik deleted the perf-read-lexer branch September 30, 2026 09:53
@staabm

staabm commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

PHPUnit, when generating code-coverage, also does some PhpToken processing via https://github.com/theseer/tokenizer

maybe you find similar opportunities there

@nikic

nikic commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Is it actually true? PhpToken uses normal properties, it does not intercept the property handlers, so I don't see why it should be slow.

@staabm

staabm commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

@nikic so you say what is faster now is that we have a variable instead of a property-fetch? its unrelated to the fact of userland class vs. native class?

@samsonasik

Copy link
Copy Markdown
Contributor Author

Reading $token->id performs an object-property lookup each time.

Subsequent $id reads use a local variable, which PHP accesses more cheaply than an object property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants