Skip to content

Commit 26638ce

Browse files
adamzieladamziel
committed
Restore instanceof WP_Parser via composition + trait
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.
1 parent 4f5c4a2 commit 26638ce

5 files changed

Lines changed: 242 additions & 1 deletion

File tree

‎.github/workflows/parser-delegation-perf.yml‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
name: Parser Delegation Perf
2+
3+
# Measures the wall-time cost of routing the public parser API through a
4+
# composed `WP_MySQL_Native_Parser` instance (this PR) instead of letting
5+
# `WP_MySQL_Parser` directly extend the Rust class (PR base). The parse
6+
# benchmark walks the full MySQL server-suite corpus on both branches —
7+
# the delta is the per-call delegation cost.
8+
9+
on:
10+
push:
11+
paths:
12+
- '.github/workflows/parser-delegation-perf.yml'
13+
- 'packages/mysql-on-sqlite/src/mysql/native/**'
14+
- 'packages/mysql-on-sqlite/tests/tools/run-parser-benchmark.php'
15+
- 'packages/php-ext-wp-mysql-parser/**'
16+
pull_request:
17+
paths:
18+
- '.github/workflows/parser-delegation-perf.yml'
19+
- 'packages/mysql-on-sqlite/src/mysql/native/**'
20+
- 'packages/mysql-on-sqlite/tests/tools/run-parser-benchmark.php'
21+
- 'packages/php-ext-wp-mysql-parser/**'
22+
workflow_dispatch:
23+
24+
concurrency:
25+
group: ${{ github.workflow }}-${{ github.ref }}
26+
cancel-in-progress: true
27+
28+
jobs:
29+
perf:
30+
name: Parser delegation benchmark
31+
runs-on: ubuntu-latest
32+
timeout-minutes: 25
33+
34+
steps:
35+
- name: Checkout repository
36+
uses: actions/checkout@v4
37+
38+
- name: Set up PHP
39+
uses: shivammathur/setup-php@v2
40+
with:
41+
php-version: '8.2'
42+
coverage: none
43+
44+
- name: Set up Rust
45+
uses: dtolnay/rust-toolchain@stable
46+
47+
- name: Install native build dependencies
48+
run: |
49+
sudo apt-get update
50+
sudo apt-get install -y libclang-dev
51+
echo "PHP_CONFIG=$(command -v php-config)" >> "$GITHUB_ENV"
52+
LIBCLANG_SO="$(find /usr/lib -name 'libclang.so*' | head -n 1)"
53+
echo "LIBCLANG_PATH=$(dirname "$LIBCLANG_SO")" >> "$GITHUB_ENV"
54+
55+
- name: Install Composer dependencies (root)
56+
uses: ramsey/composer-install@v3
57+
with:
58+
ignore-cache: "yes"
59+
composer-options: "--optimize-autoloader"
60+
61+
- name: Install Composer dependencies (mysql-on-sqlite)
62+
uses: ramsey/composer-install@v3
63+
with:
64+
working-directory: packages/mysql-on-sqlite
65+
ignore-cache: "yes"
66+
composer-options: "--optimize-autoloader"
67+
68+
- name: Download MySQL test query corpus
69+
working-directory: packages/mysql-on-sqlite
70+
run: bash tests/tools/mysql-download-tests.sh || true
71+
72+
- name: Build parser extension (release)
73+
run: cargo build --release
74+
working-directory: packages/php-ext-wp-mysql-parser
75+
76+
- name: Locate built extension
77+
run: |
78+
EXT="$GITHUB_WORKSPACE/packages/php-ext-wp-mysql-parser/target/release/libwp_mysql_parser.so"
79+
test -f "$EXT" || { echo "Extension not built at $EXT"; exit 1; }
80+
echo "NATIVE_EXT=$EXT" >> "$GITHUB_ENV"
81+
82+
- name: Benchmark — this PR (trait delegation)
83+
working-directory: packages/mysql-on-sqlite
84+
run: |
85+
for i in 1 2 3; do
86+
echo "=== run $i ==="
87+
php -d extension="$NATIVE_EXT" tests/tools/run-parser-benchmark.php
88+
done | tee "$GITHUB_WORKSPACE/this-pr.txt"
89+
90+
- name: Check out baseline (PR base, direct extends)
91+
run: |
92+
git fetch --no-tags --depth=1 origin codex/native-lazy-ast-facade
93+
git worktree add ../baseline FETCH_HEAD
94+
95+
- name: Install Composer dependencies (baseline)
96+
uses: ramsey/composer-install@v3
97+
with:
98+
working-directory: ../baseline/packages/mysql-on-sqlite
99+
ignore-cache: "yes"
100+
composer-options: "--optimize-autoloader"
101+
102+
- name: Build baseline parser extension (release)
103+
run: cargo build --release
104+
working-directory: ../baseline/packages/php-ext-wp-mysql-parser
105+
106+
- name: Stage corpus into baseline
107+
run: |
108+
mkdir -p ../baseline/packages/mysql-on-sqlite/tests/mysql/data
109+
cp packages/mysql-on-sqlite/tests/mysql/data/*.csv \
110+
../baseline/packages/mysql-on-sqlite/tests/mysql/data/ 2>/dev/null || true
111+
112+
- name: Benchmark — baseline (direct extends)
113+
working-directory: ../baseline/packages/mysql-on-sqlite
114+
run: |
115+
BASE_EXT="$(realpath ../../packages/php-ext-wp-mysql-parser/target/release/libwp_mysql_parser.so)"
116+
for i in 1 2 3; do
117+
echo "=== run $i ==="
118+
php -d extension="$BASE_EXT" tests/tools/run-parser-benchmark.php
119+
done | tee "$GITHUB_WORKSPACE/baseline.txt"
120+
121+
- name: Summarize
122+
if: always()
123+
run: |
124+
{
125+
echo '### Parser delegation perf'
126+
echo
127+
echo '#### This PR (`use WP_MySQL_Native_Parser_Impl;`)'
128+
echo '```'
129+
cat "$GITHUB_WORKSPACE/this-pr.txt" 2>/dev/null || echo 'no output'
130+
echo '```'
131+
echo
132+
echo '#### Baseline (`extends WP_MySQL_Native_Parser`)'
133+
echo '```'
134+
cat "$GITHUB_WORKSPACE/baseline.txt" 2>/dev/null || echo 'no output'
135+
echo '```'
136+
} >> "$GITHUB_STEP_SUMMARY"
137+
138+
- name: Upload raw output
139+
if: always()
140+
uses: actions/upload-artifact@v4
141+
with:
142+
name: parser-delegation-perf-${{ github.run_id }}
143+
path: |
144+
this-pr.txt
145+
baseline.txt
146+
if-no-files-found: warn

‎packages/mysql-on-sqlite/src/load.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
if ( class_exists( 'WP_MySQL_Native_Parser', false ) ) {
2929
require_once __DIR__ . '/mysql/native/mysql-rust-bridge.php';
3030
require_once __DIR__ . '/mysql/native/class-wp-mysql-native-parser-node.php';
31+
require_once __DIR__ . '/mysql/native/trait-wp-mysql-native-parser-impl.php';
3132
require_once __DIR__ . '/mysql/native/class-wp-mysql-parser.php';
3233
} else {
3334
require_once __DIR__ . '/mysql/class-wp-mysql-parser.php';

‎packages/mysql-on-sqlite/src/mysql/native/class-wp-mysql-parser.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
11
<?php
22

3-
class WP_MySQL_Parser extends WP_MySQL_Native_Parser {}
3+
/**
4+
* Native-mode public parser entry point.
5+
*
6+
* Always extends the pure-PHP `WP_Parser` so `$parser instanceof WP_Parser`
7+
* keeps working for callers regardless of whether the Rust extension is
8+
* loaded. The actual parsing work is delegated to a composed
9+
* `WP_MySQL_Native_Parser` instance via the `WP_MySQL_Native_Parser_Impl`
10+
* trait — see that file for the per-method delegation.
11+
*/
12+
class WP_MySQL_Parser extends WP_Parser {
13+
use WP_MySQL_Native_Parser_Impl;
14+
}

‎packages/mysql-on-sqlite/src/mysql/native/trait-wp-mysql-native-parser-impl.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/**
4+
* Native-mode `WP_MySQL_Parser` implementation, delivered as a trait.
5+
*
6+
* The class that uses this trait (`WP_MySQL_Parser` in native mode)
7+
* extends the pure-PHP `WP_Parser` so callers' `instanceof WP_Parser`
8+
* checks keep working, while the actual parsing work is delegated to
9+
* the Rust-registered `WP_MySQL_Native_Parser` instance held in
10+
* `$this->native`. `WP_Parser`'s state (`$grammar`, `$tokens`,
11+
* `$position`) stays inert in native mode — the trait's overrides
12+
* never read it.
13+
*
14+
* Adding a public method here is enough to plumb a new public method
15+
* through to the native parser; the using class does not need touching.
16+
*/
17+
trait WP_MySQL_Native_Parser_Impl {
18+
/**
19+
* @var WP_MySQL_Native_Parser
20+
*/
21+
private $native;
22+
23+
public function __construct( WP_Parser_Grammar $grammar, array $tokens ) {
24+
parent::__construct( $grammar, $tokens );
25+
$this->native = new WP_MySQL_Native_Parser( $grammar, $tokens );
26+
}
27+
28+
public function reset_tokens( array $tokens ): void {
29+
$this->native->reset_tokens( $tokens );
30+
}
31+
32+
public function next_query(): bool {
33+
return $this->native->next_query();
34+
}
35+
36+
public function get_query_ast(): ?WP_Parser_Node {
37+
return $this->native->get_query_ast();
38+
}
39+
40+
public function parse() {
41+
return $this->native->parse();
42+
}
43+
}

‎packages/mysql-on-sqlite/tests/mysql/native/WP_MySQL_Parser_Instanceof_Tests.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
use PHPUnit\Framework\TestCase;
4+
5+
/**
6+
* `WP_MySQL_Parser instanceof WP_Parser` must hold in both modes.
7+
*
8+
* Pre-PR, the native-mode `WP_MySQL_Parser` was `extends WP_MySQL_Native_Parser`
9+
* (a Rust-registered class with no `WP_Parser` in its chain), so existing
10+
* downstream code doing `if ($parser instanceof WP_Parser)` silently
11+
* skipped the parser when the extension was loaded. This test pins the
12+
* contract for both modes.
13+
*/
14+
class WP_MySQL_Parser_Instanceof_Tests extends TestCase {
15+
16+
public function test_parser_is_instance_of_wp_parser(): void {
17+
$grammar = new WP_Parser_Grammar( include __DIR__ . '/../../../src/mysql/mysql-grammar.php' );
18+
$lexer = new WP_MySQL_Lexer( 'SELECT 1' );
19+
$tokens = $lexer instanceof WP_MySQL_Native_Lexer
20+
? $lexer->native_token_stream()
21+
: $lexer->remaining_tokens();
22+
$parser = new WP_MySQL_Parser( $grammar, $tokens );
23+
24+
$this->assertInstanceOf( WP_Parser::class, $parser );
25+
$this->assertInstanceOf( WP_MySQL_Parser::class, $parser );
26+
}
27+
28+
public function test_parser_returns_an_ast(): void {
29+
$grammar = new WP_Parser_Grammar( include __DIR__ . '/../../../src/mysql/mysql-grammar.php' );
30+
$lexer = new WP_MySQL_Lexer( 'SELECT 1 + 2' );
31+
$tokens = $lexer instanceof WP_MySQL_Native_Lexer
32+
? $lexer->native_token_stream()
33+
: $lexer->remaining_tokens();
34+
$parser = new WP_MySQL_Parser( $grammar, $tokens );
35+
36+
$ast = $parser->parse();
37+
$this->assertNotNull( $ast );
38+
$this->assertInstanceOf( WP_Parser_Node::class, $ast );
39+
}
40+
}

0 commit comments

Comments
 (0)