Optimize preceding-sibling, following-sibling, following, and descendant axes with an unified single mechanism by tompng · Pull Request #341 · ruby/rexml

tompng · 2026-06-27T16:25:45Z

GitHub: FixGH-331

Unify the positional-predicate handling of the descendant, descendant-or-self,
following, following-sibling, and preceding-sibling axes into a single
event-stream mechanism (sequence_positional_scan).

Detail

Consider xpath like this: anchor/axis::*[test-predicate][simple-positional-predicate].
Some xpath axes (descendant, descendant-or-self, following, preceding-sibling, following-sibling) have same common structure:

Anchor start point, anchor end point, and nodes that passed test predicate lines up in a single sequence
Anchor start and end are nested and doesn't cross over

Example of descendant:

anchor1-start
node1
node2
anchor2-start
node3
node4
anchor2-end
node5
anchor3-start
node6
anchor3-end
node7
anchor1-end
# Nodesets: [
# [node1, node2, node3, node4, node5, node6, node7], # from anchor1
# [node3, node4], # from anchor2
# [node6] # from anchor3
# ]

The above sequence and anchor ranges can be represented as event stream like this:

[:push,node1,node2,:push,node3,node4,:pop,node5,:push,node6,:pop,node7,:pop]

Events are:

:push: Add a new anchor point
:pop: Remove last anchor point
node: Add a node that passed test predicate

Axis scanner of following, descendant, descendant-or-self, preceding-sibling and following-sibling will construct an event stream and passes it to sequence_positional_scan which implements positional predicate optimization.
Optimization logic in sequence_positional_scan is basically the same as the one implemented in preceding/following-sibling before.

Unchanged axes (preceding, ancestor)

preceding and ancestor axes are intentionally left on the existing path. Unlike following axis, anchor-exclusion semantics of preceding axis don't fit the nested push/pop model.

Benchmark

XPath in this benchmark is specially crafted to precisely measure axis scan without noise.
Using * and count() will avoid O(n^2) sort and namespace lookup which may be fixed in a near future.

xml='<a>'*200+'<a/>'*200+'</a>'*200xpaths=["count(//*/descendant::*[position()<10])","count(//*/preceding-sibling::*[position()<10])","count(//*/following-sibling::*[position()<10])","count(//*/following::*[position()<10])",]xpaths.eachdo |xpath|
putsxpathdoc=REXML::Document.new(xml)t=Time.now;p[REXML::XPath.match(doc,xpath),Time.now-t]doc=Nokogiri::XML.parse(xml)t=Time.now;p[doc.xpath(xpath),Time.now-t]end

scenario	REXML(master)	REXML(this PR)	Nokogiri
`descendant::*[position()<10]`	0.032287 sec	0.002089 sec	0.010074 sec
`preceding-sibling::*[position()<10]`	0.001075 sec	0.001239 sec	0.00265 sec
`following-sibling::*[position()<10]`	0.000892 sec	0.001041 sec	0.002627 sec
`following::*[position()<10]`	0.110464 sec	0.000942 sec	0.002706 sec

In this example, XPath match is faster than Nokogiri, mainly because nokogiri doesn't optimize [position()<N].

Copilot

Pull request overview

This PR refactors XPath axis scanning to unify positional-predicate optimization across descendant, descendant-or-self, following, and sibling axes via a shared event-stream mechanism (sequence_positional_scan), extending support to simple last()/last()-N forms and adding targeted regression tests.

Changes:

Extend simple positional predicate detection to cover last() and last()-N, producing 0-based forward/reverse index operators.
Replace per-axis positional handling with a shared event-stream scan (sequence_positional_scan) and integrate it into descendant/following/sibling axis scanners.
Add tests covering out-of-range last()-N, and positional behaviors across descendant/descendant-or-self/following and sibling axes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
test/xpath/test_predicate.rb	Adds regression tests for out-of-range `last()-N` predicates.
test/xpath/test_base.rb	Adds positional tests for descendant/descendant-or-self/following across multiple anchor shapes.
test/xpath/test_axis_preceding_sibling.rb	Expands sibling-axis tests to cover additional position expressions and `last()-N` variants.
lib/rexml/xpath_parser.rb	Implements unified positional predicate parsing and the shared event-stream scanning mechanism; updates axes to use it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 def following(nodeset, tester, selector)
- nodesets = nodeset.select {|node| node.respond_to?(:parent) }.map do |node|
- following_nodes(node)
+ anchors = Set.new.compare_by_identity.replace(nodeset)
+ events = []
+ descendant_traverse_event(nodeset.first.document || nodeset.first.root) do |type, node|
+ events << :push if type == :leave && anchors.include?(node)
+ events << node if !events.empty? && type == :enter && tester.call(node)
 end
- non_optimized_nodesets_select(nodesets, tester, selector)
- end
-
- def following_nodes(node)
- followings = []
- following_node = next_sibling_node(node)
- while following_node
- followings << following_node
- following_node = following_node_of(following_node)
- end
- followings
- end
-
- def following_node_of( node )
- return node.children[0] if node.kind_of?(Element) and node.children.size > 0
-
- next_sibling_node(node)
- end
-
- def next_sibling_node(node)
- psn = node.next_sibling_node
- while psn.nil?
- return nil if node.parent.nil? or node.parent.class == Document
- node = node.parent
- psn = node.next_sibling_node
- end
- psn
+ anchors.size.times { events << :pop }
+ sequence_positional_scan(events, selector)
 end


naitoh · 2026-07-07T11:50:44Z

@tompng
Thank you.
I think this is a great improvement.

However, please check the following.

This PR is in conflict.
It appears there is a performance degradation of about 9% in //c.

$ benchmark-driver benchmark/xpath_latest.yaml
Calculating -------------------------------------
before(master) after before(master)(YJIT) after(YJIT) REXML::XPath.match(doc, '//c') 93.025 84.225 126.617 116.110 i/s - 100.000 timesin 1.074978s 1.187298s 0.789785s 0.861253s
Comparison:
REXML::XPath.match(doc, '//c')
before(master)(YJIT): 126.6 i/s after(YJIT): 116.1 i/s - 1.09x slower
before(master): 93.0 i/s - 1.36x slower
after: 84.2 i/s - 1.50x slower

Benchmark Code

$ git checkout master
$ rake build
$ gem install pkg/rexml-3.4.5.gem

benchmark/xpath_latest.yamlloop_count: 100contexts:
- name: before(master)gems:
rexml: 3.4.5require: falseprelude: require 'rexml'
- name: afterprelude: | $LOAD_PATH.unshift(File.expand_path("lib")) require 'rexml' - name: before(master)(YJIT)gems:
rexml: 3.4.5require: falseprelude: | require 'rexml' RubyVM::YJIT.enable - name: after(YJIT)prelude: | $LOAD_PATH.unshift(File.expand_path("lib")) require 'rexml' RubyVM::YJIT.enableprelude: | require 'rexml/document' xml = "<r>" + "<g><h>#{'<b><c/></b>' * 50}</h></g>" * 40 + "</r>" doc = REXML::Document.new(xml)benchmark:
"REXML::XPath.match(doc, '//c')": REXML::XPath.match(doc, "//c")

…ant axes with a unified single mechanism

tompng · 2026-07-07T14:00:37Z

Conflict: rebased.
This removes each selector = :uniq optimization and replace it to a generic one. Special optimization is always faster than generic optimization, but will introduce complexity.
I think simplicity/maintainability is important, and compared to that, +9% is a small change. Also note that this is already incomparably faster than the latest released version due to a better time complexity.
How about reconsider adding special optimization after this pull request, and compare if it's speed up is worth adding complexity for each axis.

naitoh · 2026-07-11T13:25:12Z

Conflict: rebased.

Thanks.

How about reconsider adding special optimization after this pull request, and compare if it's speed up is worth adding complexity for each axis.

OK.
I see.

naitoh

I was able to understand the new algorithm.
The steps are well-organized, and I think it’s easier to understand than before.
Thank you very much.

CopilotAI review requested due to automatic review settings June 27, 2026 16:25

Copilotstarted reviewing on behalf of tompng June 27, 2026 16:26 View session

CopilotAI reviewed Jun 27, 2026
View reviewed changes

tompng changed the title ~~Optimize preceding-sibling, following-sibling, following, and descendant axes with a unified single mechanism~~Optimize preceding-sibling, following-sibling, following, and descendant axes with an unified single mechanismJul 4, 2026

Optimize preceding-sibling, following-sibling, following, and descend…
b3b7ba5
…ant axes with a unified single mechanism

tompng force-pushed the xpath_step_optimize_more branch from 6e9b376 to b3b7ba5Compare July 7, 2026 12:59

naitoh approved these changes Jul 11, 2026
View reviewed changes

naitoh merged commit 73aa07a into ruby:masterJul 11, 2026
70 of 71 checks passed

tompng deleted the xpath_step_optimize_more branch July 11, 2026 14:28

Navigation Menu

Uh oh!

Optimize preceding-sibling, following-sibling, following, and descendant axes with an unified single mechanism - #341

Optimize preceding-sibling, following-sibling, following, and descendant axes with an unified single mechanism#341
naitoh merged 1 commit into
ruby:masterfrom
tompng:xpath_step_optimize_more

tompng commented Jun 27, 2026•
edited by naitoh
Loading

Uh oh!

CopilotAI left a comment

Uh oh!

naitoh commented Jul 7, 2026

Uh oh!

tompng commented Jul 7, 2026

Uh oh!

naitoh commented Jul 11, 2026

Uh oh!

naitoh left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tompng commented Jun 27, 2026• edited by naitoh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Detail

Unchanged axes (preceding, ancestor)

Benchmark

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

naitoh commented Jul 7, 2026

Benchmark Code

Uh oh!

tompng commented Jul 7, 2026

Uh oh!

naitoh commented Jul 11, 2026

Uh oh!

naitoh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tompng commented Jun 27, 2026•
edited by naitoh
Loading