Restore instanceof WP_Parser for the native parser#393
Restore instanceof WP_Parser for the native parser#393adamziel merged 2 commits intocodex/native-lazy-ast-facadefrom
instanceof WP_Parser for the native parser#393Conversation
Pre-PR, the native-mode WP_MySQL_Parser was `extends WP_MySQL_Native_Parser` (a Rust class with no WP_Parser in its chain), so callers' existing `if ($parser instanceof WP_Parser)` checks silently dropped the parser when the extension was loaded. This restores the contract by always extending WP_Parser and pulling the native-mode behaviour in via a trait. The trait owns the composed WP_MySQL_Native_Parser instance and the four-method delegation surface; the class file itself is two lines. Adding a public method now means adding it to the trait — no further wiring. WP_Parser's protected state ($grammar, $tokens, $position) stays inert in native mode: parent::__construct still initialises it, but the trait's overrides never read it. Adds a regression test for the instanceof contract and a CI workflow that benchmarks parse() across the full MySQL server-suite corpus on this branch and on the PR base, on the same runner. The delta is the delegation overhead — one extra method-call frame per query.
The SQLite driver passes a WP_MySQL_Native_Token_Stream object as the $tokens argument when the extension is loaded; pre-PR this worked because the parent class was the Rust WP_MySQL_Native_Parser, which accepts either an array or the stream object. The trait's constructor needs the same flexibility — drop the array typehint and pass an empty array to WP_Parser::__construct, whose tokens/position state is inert in native mode.
Parser delegation perfCI run 25216548485. Three back-to-back runs of
Median delta: −0.26s (−1.7%) duration, +1.9% qps — i.e. the trait/composition variant measured faster than baseline. That's not a real win; the trait adds an extra The honest summary: the delegation cost is invisible at this granularity, which is what we wanted to confirm before fixing the Other CI
|
1 participant
Addresses Jan's note on #381: in native mode,
new WP_MySQL_Parser(...) instanceof WP_Parserreturns false because the native-mode class extends the Rust-registeredWP_MySQL_Native_Parser, which has noWP_Parserin its chain. Existing downstream code doingif ($parser instanceof WP_Parser)silently skipped the parser whenever the extension was loaded.This restores the contract by always extending the pure-PHP
WP_Parserand pulling the native-mode behaviour in via a trait:WP_MySQL_Native_Parser_Implowns the composedWP_MySQL_Native_Parserinstance and the four-method delegation surface (parse,next_query,get_query_ast,reset_tokens).WP_Parser's protected state ($grammar,$tokens,$position) is initialised byparent::__constructand stays inert — the trait's overrides never read it.Adding a public method later means adding it to the trait — the class file itself is two lines and doesn't need touching.
Why a trait, not a private property?
A bare property would also work, but the trait keeps the class file expressing only the routing decision (
extends WP_Parser+use Rust_Implementation;). The implementation lives in one place, symmetric to where a future PHP-mode trait could live if we ever want to mirror the structure. Behaviour-wise the two are equivalent.Performance
The trait adds one extra method-call frame per public-API call. The public API is
parse(),next_query(),get_query_ast(),reset_tokens()— called once per query. The actual parsing work happens inside the native call, so the delegation overhead is a small constant per query, not a multiplier on the parsing work.The
Parser Delegation Perfworkflow runstests/tools/run-parser-benchmark.php(parses the full MySQL server-suite corpus, ~70k queries) three times on this PR and three times on the PR base, on the same runner, with the extension loaded both times. The comparison goes into the job summary on every push.Test plan
WP_MySQL_Parser_Instanceof_Testsconfirminstanceof WP_Parserandinstanceof WP_MySQL_Parserboth hold.Parser Delegation Perfworkflow shows the delegation cost is within noise.