Preserve child wrapper identity in native AST facade

adamziel · adamziel · commit 61a00fa4f670 · 2026-04-30T19:08:51.000+02:00
The native parser extension constructs a fresh WP_MySQL_Native_Parser_Node
on every accessor call, so two reads of the same logical node returned
different PHP objects and any state a caller attached to the first wrapper
was invisible through the second. WP_Parser_Node has always given callers
stable child identity, and the lazy native facade is meant to keep that
contract intact — this restores it.

A per-AST identity map is created lazily on the root and shared by every
interned wrapper. Each accessor that returns a node looks the index up
in the map and returns the canonical instance, discarding the freshly
constructed one. Materialization pulls children through the same map so
mutations a caller made through get_first_child_node() before triggering
append_child() survive into $this-&gt;children.

Adds a regression test that exercises identity across child, descendant,
and post-materialization reads, plus a walk benchmark and a CI workflow
that reports parse + walk time and peak memory for the PHP and native
paths so the cache cost is measurable on every PR.
diff --git a/.github/workflows/native-ast-perf.yml b/.github/workflows/native-ast-perf.yml
@@ -0,0 +1,118 @@
+name: Native AST Walk Perf
+
+on:
+  push:
+    paths:
+      - '.github/workflows/native-ast-perf.yml'
+      - 'packages/mysql-on-sqlite/src/mysql/native/**'
+      - 'packages/mysql-on-sqlite/tests/tools/run-native-ast-walk-benchmark.php'
+      - 'packages/php-ext-wp-mysql-parser/**'
+  pull_request:
+    paths:
+      - '.github/workflows/native-ast-perf.yml'
+      - 'packages/mysql-on-sqlite/src/mysql/native/**'
+      - 'packages/mysql-on-sqlite/tests/tools/run-native-ast-walk-benchmark.php'
+      - 'packages/php-ext-wp-mysql-parser/**'
+  workflow_dispatch:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  perf:
+    name: Native AST walk benchmark
+    runs-on: ubuntu-latest
+    timeout-minutes: 25
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Set up PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: '8.2'
+          coverage: none
+
+      - name: Set up Rust
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Install native build dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y libclang-dev
+          echo "PHP_CONFIG=$(command -v php-config)" >> "$GITHUB_ENV"
+          LIBCLANG_SO="$(find /usr/lib -name 'libclang.so*' | head -n 1)"
+          echo "LIBCLANG_PATH=$(dirname "$LIBCLANG_SO")" >> "$GITHUB_ENV"
+
+      - name: Install Composer dependencies (root)
+        uses: ramsey/composer-install@v3
+        with:
+          ignore-cache: "yes"
+          composer-options: "--optimize-autoloader"
+
+      - name: Install Composer dependencies (mysql-on-sqlite)
+        uses: ramsey/composer-install@v3
+        with:
+          working-directory: packages/mysql-on-sqlite
+          ignore-cache: "yes"
+          composer-options: "--optimize-autoloader"
+
+      - name: Download MySQL test query corpus
+        working-directory: packages/mysql-on-sqlite
+        run: bash tests/tools/mysql-download-tests.sh || true
+
+      - name: Build parser extension (release)
+        run: cargo build --release
+        working-directory: packages/php-ext-wp-mysql-parser
+
+      - name: Locate built extension
+        run: |
+          EXT="$GITHUB_WORKSPACE/packages/php-ext-wp-mysql-parser/target/release/libwp_mysql_parser.so"
+          test -f "$EXT" || { echo "Extension not built at $EXT"; exit 1; }
+          echo "NATIVE_EXT=$EXT" >> "$GITHUB_ENV"
+
+      - name: Benchmark — pure-PHP path (parse only)
+        working-directory: packages/mysql-on-sqlite
+        run: |
+          php tests/tools/run-native-ast-walk-benchmark.php --no-walk | tee php-parse-only.txt
+
+      - name: Benchmark — pure-PHP path (walk)
+        working-directory: packages/mysql-on-sqlite
+        run: |
+          php tests/tools/run-native-ast-walk-benchmark.php | tee php-walk.txt
+
+      - name: Benchmark — native path (parse only)
+        working-directory: packages/mysql-on-sqlite
+        run: |
+          php -d extension="$NATIVE_EXT" tests/tools/run-native-ast-walk-benchmark.php --no-walk | tee native-parse-only.txt
+
+      - name: Benchmark — native path (walk, with identity cache)
+        working-directory: packages/mysql-on-sqlite
+        run: |
+          php -d extension="$NATIVE_EXT" tests/tools/run-native-ast-walk-benchmark.php | tee native-walk.txt
+
+      - name: Summarize
+        if: always()
+        working-directory: packages/mysql-on-sqlite
+        run: |
+          {
+            echo '### Native AST walk perf'
+            echo
+            echo '| scenario | result |'
+            echo '|---|---|'
+            for f in php-parse-only.txt php-walk.txt native-parse-only.txt native-walk.txt; do
+              [ -f "$f" ] || continue
+              line="$(cat "$f")"
+              echo "| ${f%.txt} | \`$line\` |"
+            done
+          } >> "$GITHUB_STEP_SUMMARY"
+
+      - name: Upload raw output
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: native-ast-perf-${{ github.run_id }}
+          path: packages/mysql-on-sqlite/*.txt
+          if-no-files-found: warn
diff --git a/packages/mysql-on-sqlite/src/load.php b/packages/mysql-on-sqlite/src/load.php
@@ -27,6 +27,7 @@
 
 if ( class_exists( 'WP_MySQL_Native_Parser', false ) ) {
 	require_once __DIR__ . '/mysql/native/mysql-rust-bridge.php';
+	require_once __DIR__ . '/mysql/native/class-wp-mysql-native-ast-cache.php';
 	require_once __DIR__ . '/mysql/native/class-wp-mysql-native-parser-node.php';
 	require_once __DIR__ . '/mysql/native/class-wp-mysql-parser.php';
 } else {
diff --git a/packages/mysql-on-sqlite/src/mysql/native/class-wp-mysql-native-ast-cache.php b/packages/mysql-on-sqlite/src/mysql/native/class-wp-mysql-native-ast-cache.php
@@ -0,0 +1,23 @@
+<?php
+
+/**
+ * Per-AST identity map for native parser node wrappers.
+ *
+ * The native parser extension constructs a fresh WP_MySQL_Native_Parser_Node
+ * every time it returns a child or descendant. Without an identity map, two
+ * reads of the same logical node would yield distinct PHP objects, breaking
+ * the WP_Parser_Node contract that callers can mutate a child in place and
+ * see the mutation again when they walk the tree later.
+ *
+ * One cache is created lazily on the root node and shared by reference with
+ * every wrapper interned through it. Lookup is keyed by the Rust-side
+ * `node_index`, which is stable for the lifetime of the AST.
+ */
+class WP_MySQL_Native_AST_Cache {
+	/**
+	 * Map of native node index => WP_MySQL_Native_Parser_Node.
+	 *
+	 * @var array<int, WP_MySQL_Native_Parser_Node>
+	 */
+	public $nodes = array();
+}
diff --git a/packages/mysql-on-sqlite/src/mysql/native/class-wp-mysql-native-parser-node.php b/packages/mysql-on-sqlite/src/mysql/native/class-wp-mysql-native-parser-node.php
@@ -8,19 +8,49 @@
  * caller actually walks the tree. On the first mutation (append_child or
  * merge_fragment), the node materializes its children into the inherited
  * `$children` array and behaves like a plain WP_Parser_Node from then on.
+ *
+ * Wrappers returned by accessors are interned through a per-AST identity
+ * map (WP_MySQL_Native_AST_Cache) so two reads of the same logical node
+ * yield the same PHP instance. This preserves the WP_Parser_Node contract
+ * that mutations performed on a child via `get_first_child_node()` remain
+ * visible when the same child is reached again, including after the parent
+ * has materialized.
  */
 class WP_MySQL_Native_Parser_Node extends WP_Parser_Node {
 	private $native_ast        = null;
 	private $native_node_index = null;
 	private $was_mutated       = false;
 
+	/**
+	 * Per-AST identity map shared between every interned wrapper.
+	 *
+	 * Created lazily on the first child access; the root wrapper is the
+	 * first entry. Children inherit the same cache instance by reference.
+	 *
+	 * @var WP_MySQL_Native_AST_Cache|null
+	 */
+	private $cache = null;
+
 	public function __construct( $rule_id, $rule_name, $native_ast = null, $native_node_index = null ) {
 		parent::__construct( $rule_id, $rule_name );
 
 		$this->native_ast        = $native_ast;
 		$this->native_node_index = $native_node_index;
 	}
 
+	/**
+	 * Native node index in the Rust-owned arena.
+	 *
+	 * Exposed so the identity cache can key on it. Returns null after
+	 * the wrapper has materialized — at that point the node is detached
+	 * from the native arena and behaves like a plain WP_Parser_Node.
+	 *
+	 * @return int|null
+	 */
+	public function get_native_node_index(): ?int {
+		return $this->native_node_index;
+	}
+
 	/** @inheritDoc */
 	public function append_child( $node ) {
 		$this->materialize_native_children();
@@ -65,15 +95,15 @@ public function get_first_child() {
 		if ( $this->was_mutated() ) {
 			return parent::get_first_child();
 		}
-		return wp_sqlite_mysql_native_ast_get_first_child( $this->native_ast, $this->native_node_index );
+		return $this->intern( wp_sqlite_mysql_native_ast_get_first_child( $this->native_ast, $this->native_node_index ) );
 	}
 
 	/** @inheritDoc */
 	public function get_first_child_node( ?string $rule_name = null ): ?WP_Parser_Node {
 		if ( $this->was_mutated() ) {
 			return parent::get_first_child_node( $rule_name );
 		}
-		return wp_sqlite_mysql_native_ast_get_first_child_node( $this->native_ast, $this->native_node_index, $rule_name );
+		return $this->intern( wp_sqlite_mysql_native_ast_get_first_child_node( $this->native_ast, $this->native_node_index, $rule_name ) );
 	}
 
 	/** @inheritDoc */
@@ -89,7 +119,7 @@ public function get_first_descendant_node( ?string $rule_name = null ): ?WP_Pars
 		if ( $this->was_mutated() ) {
 			return parent::get_first_descendant_node( $rule_name );
 		}
-		return wp_sqlite_mysql_native_ast_get_first_descendant_node( $this->native_ast, $this->native_node_index, $rule_name );
+		return $this->intern( wp_sqlite_mysql_native_ast_get_first_descendant_node( $this->native_ast, $this->native_node_index, $rule_name ) );
 	}
 
 	/** @inheritDoc */
@@ -105,15 +135,15 @@ public function get_children(): array {
 		if ( $this->was_mutated() ) {
 			return parent::get_children();
 		}
-		return wp_sqlite_mysql_native_ast_get_children( $this->native_ast, $this->native_node_index );
+		return $this->intern_all( wp_sqlite_mysql_native_ast_get_children( $this->native_ast, $this->native_node_index ) );
 	}
 
 	/** @inheritDoc */
 	public function get_child_nodes( ?string $rule_name = null ): array {
 		if ( $this->was_mutated() ) {
 			return parent::get_child_nodes( $rule_name );
 		}
-		return wp_sqlite_mysql_native_ast_get_child_nodes( $this->native_ast, $this->native_node_index, $rule_name );
+		return $this->intern_all( wp_sqlite_mysql_native_ast_get_child_nodes( $this->native_ast, $this->native_node_index, $rule_name ) );
 	}
 
 	/** @inheritDoc */
@@ -129,15 +159,15 @@ public function get_descendants(): array {
 		if ( $this->was_mutated() ) {
 			return parent::get_descendants();
 		}
-		return wp_sqlite_mysql_native_ast_get_descendants( $this->native_ast, $this->native_node_index );
+		return $this->intern_all( wp_sqlite_mysql_native_ast_get_descendants( $this->native_ast, $this->native_node_index ) );
 	}
 
 	/** @inheritDoc */
 	public function get_descendant_nodes( ?string $rule_name = null ): array {
 		if ( $this->was_mutated() ) {
 			return parent::get_descendant_nodes( $rule_name );
 		}
-		return wp_sqlite_mysql_native_ast_get_descendant_nodes( $this->native_ast, $this->native_node_index, $rule_name );
+		return $this->intern_all( wp_sqlite_mysql_native_ast_get_descendant_nodes( $this->native_ast, $this->native_node_index, $rule_name ) );
 	}
 
 	/** @inheritDoc */
@@ -168,12 +198,78 @@ private function was_mutated(): bool {
 		return $this->was_mutated;
 	}
 
+	/**
+	 * Intern a single accessor return value through the per-AST cache.
+	 *
+	 * Tokens and nulls pass through untouched. Native node wrappers are
+	 * keyed on their `native_node_index`: on cache miss, the freshly
+	 * constructed wrapper is stored and given the cache reference; on
+	 * cache hit, the canonical instance is returned and the new wrapper
+	 * is discarded so callers see stable identity and surviving mutations.
+	 *
+	 * @param mixed $value Return value from the Rust bridge.
+	 * @return mixed
+	 */
+	private function intern( $value ) {
+		if ( ! $value instanceof WP_MySQL_Native_Parser_Node ) {
+			return $value;
+		}
+
+		$cache = $this->ensure_cache();
+		$index = $value->native_node_index;
+		if ( null === $index ) {
+			return $value;
+		}
+		if ( isset( $cache->nodes[ $index ] ) ) {
+			return $cache->nodes[ $index ];
+		}
+		$value->cache          = $cache;
+		$cache->nodes[ $index ] = $value;
+		return $value;
+	}
+
+	/**
+	 * Intern every entry in an accessor return array.
+	 *
+	 * @param array $values
+	 * @return array
+	 */
+	private function intern_all( array $values ): array {
+		foreach ( $values as $i => $value ) {
+			$values[ $i ] = $this->intern( $value );
+		}
+		return $values;
+	}
+
+	/**
+	 * Lazily build (or reuse) the per-AST identity map.
+	 *
+	 * The root wrapper is constructed without a cache, so the first time
+	 * any accessor needs to intern a child, it creates the cache and
+	 * registers itself as the root entry. Subsequent interns on this
+	 * wrapper or any descendant share the same cache by reference.
+	 *
+	 * @return WP_MySQL_Native_AST_Cache
+	 */
+	private function ensure_cache(): WP_MySQL_Native_AST_Cache {
+		if ( null === $this->cache ) {
+			$this->cache = new WP_MySQL_Native_AST_Cache();
+			if ( null !== $this->native_node_index ) {
+				$this->cache->nodes[ $this->native_node_index ] = $this;
+			}
+		}
+		return $this->cache;
+	}
+
 	private function materialize_native_children(): void {
 		if ( $this->was_mutated ) {
 			return;
 		}
 
-		$this->children          = wp_sqlite_mysql_native_ast_get_children( $this->native_ast, $this->native_node_index );
+		// Pull children through the cache so any wrapper a caller already
+		// mutated via get_first_child_node() etc. survives the transition
+		// into $this->children — same instance, same mutations.
+		$this->children          = $this->intern_all( wp_sqlite_mysql_native_ast_get_children( $this->native_ast, $this->native_node_index ) );
 		$this->native_ast        = null;
 		$this->native_node_index = null;
 		$this->was_mutated       = true;
diff --git a/packages/mysql-on-sqlite/tests/mysql/native/WP_MySQL_Native_Parser_Node_Identity_Tests.php b/packages/mysql-on-sqlite/tests/mysql/native/WP_MySQL_Native_Parser_Node_Identity_Tests.php
diff --git a/packages/mysql-on-sqlite/tests/tools/run-native-ast-walk-benchmark.php b/packages/mysql-on-sqlite/tests/tools/run-native-ast-walk-benchmark.php
