Use native-backed AST nodes in the Rust parser#423
Draft
adamziel wants to merge 1 commit into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
WP_MySQL_Native_Parser_Nodewrappers from the Rust parser instead of eagerly materializing the whole AST into PHP objects.HashMapwith a denseVec<Option<usize>>keyed by arena node index.--consume=none|descendantstorun-parser-benchmark.phpso lazy AST parser-only numbers can be compared with a forced full-tree traversal.Benchmarks
Local PHP 8.4.5, 69,577 MySQL server-suite queries, 3 reps each.
--consume=none)--consume=none)--consume=descendants)--consume=descendants)--consume=descendantsis a better benchmark than parser-only for lazy AST behavior because it forces PHP to touch all 4,786,376 descendants. It is still somewhat pessimistic for the SQLite translator because the translator usually reads selected subtrees rather than callingget_descendants()on the root.Profiling notes
zend_update_property, property/hash lookups, zval arrays, and object destruction.Verification
cargo fmt --checkphp -l tests/tools/run-parser-benchmark.phpphp -l src/mysql/native/class-wp-mysql-native-parser-node.phpphp -d extension="$EXT" tests/tools/verify-native-parser-extension.phpphp -d extension="$EXT" vendor/bin/phpunit tests/mysql/native/WP_MySQL_Parser_Instanceof_Tests.phpphp -d extension="$EXT" vendor/bin/phpunit tests/mysql/WP_MySQL_Server_Suite_Parser_Tests.phpphp -d extension="$EXT" vendor/bin/phpunit tests/WP_SQLite_Driver_Translation_Tests.phpCaveat
This makes
WP_Parser_Nodenon-final so the native facade can subclass it. That keeps the pure-PHP parser methods free of native checks, but may slightly reduce pure-PHP/JIT parser performance. A final-preserving native-handle design is possible, but would be more invasive.