Skip to content

Commit f2bdb4b

Browse files
committed
Optimize traversal and caching
Cache root and size lookups with proper invalidation, and reduce allocations in traversal and sibling helpers. Add a benchmark script to exercise common operations.
1 parent 08a5860 commit f2bdb4b

4 files changed

Lines changed: 156 additions & 40 deletions

File tree

lib/tree.rb

Lines changed: 67 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,13 @@ class TreeNode
127127
# Root node for the (sub)tree to which this node belongs.
128128
# A root node's root is itself.
129129
#
130-
# @note This walks the parent chain each call; if this is on a hot path,
131-
# consider caching the result at the caller.
132-
#
133130
# @return [Tree::TreeNode] Root of the (sub)tree.
134131
def root
135-
root = self
136-
root = root.parent until root.root?
137-
root
132+
return @root_cache if @root_cache
133+
134+
root_node = self
135+
root_node = root_node.parent until root_node.root?
136+
@root_cache = root_node
138137
end
139138

140139
# @!attribute [r] root?
@@ -232,9 +231,9 @@ def initialize(name, content = nil)
232231
@name = name
233232
@content = content
234233

235-
set_as_root!
236234
@children_hash = {}
237235
@children = []
236+
set_as_root!
238237
end
239238

240239
# Returns a copy of this node, with its parent and children links removed.
@@ -422,6 +421,7 @@ def add(child, at_index = -1)
422421

423422
@children_hash[child.name] = child
424423
child.parent = self
424+
invalidate_size_cache_upwards!
425425
child
426426
end
427427

@@ -514,6 +514,7 @@ def remove!(child)
514514
@children_hash.delete(child.name)
515515
@children.delete(child)
516516
child.set_as_root!
517+
invalidate_size_cache_upwards!
517518
child
518519
end
519520

@@ -526,6 +527,8 @@ def remove!(child)
526527
def parent=(parent) # :nodoc:
527528
@parent = parent
528529
@node_depth = nil
530+
clear_root_cache!
531+
invalidate_size_cache_upwards!
529532
end
530533

531534
protected :parent=, :name=
@@ -559,6 +562,7 @@ def remove_all!
559562

560563
@children_hash.clear
561564
@children.clear
565+
invalidate_size_cache_upwards!
562566
self
563567
end
564568

@@ -571,6 +575,25 @@ def set_as_root! # :nodoc:
571575

572576
protected :set_as_root!
573577

578+
def clear_root_cache!
579+
@root_cache = nil
580+
return unless @children
581+
582+
children_array.each do |child|
583+
child&.__send__(:clear_root_cache!)
584+
end
585+
end
586+
587+
def invalidate_size_cache_upwards!
588+
node = self
589+
while node
590+
node.instance_variable_set(:@node_size, nil)
591+
node = node.parent
592+
end
593+
end
594+
595+
private :clear_root_cache!, :invalidate_size_cache_upwards!
596+
574597
# Freezes all nodes in the (sub)tree rooted at this node.
575598
#
576599
# The nodes become immutable after this operation. In effect, the entire tree's
@@ -642,7 +665,7 @@ def each # :yields: node
642665

643666
yield current # and process it
644667
# Stack children of the current node at top of the stack
645-
current.children.reverse_each do |child|
668+
current.send(:children_array).reverse_each do |child|
646669
node_stack << child if child
647670
end
648671
end
@@ -676,25 +699,22 @@ def preordered_each(&block) # :yields: node
676699
def postordered_each
677700
return to_enum(:postordered_each) unless block_given?
678701

679-
# Using a marked node in order to skip adding the children of nodes that
680-
# have already been visited. This allows the stack depth to be controlled,
681-
# and also allows stateful backtracking.
682-
marked_node = Struct.new(:node, :visited)
683-
node_stack = [marked_node.new(self, false)] # Start with self
702+
node_stack = [self] # Start with self
703+
visited = [false]
684704

685705
until node_stack.empty?
686-
peek_node = node_stack[0]
687-
if peek_node.node.children? && !peek_node.visited
688-
peek_node.visited = true
689-
# Add the children to the stack. Use the marking structure.
690-
marked_children =
691-
peek_node.node.children.filter_map do |node|
692-
marked_node.new(node, false) if node
693-
end
694-
node_stack = marked_children.concat(node_stack)
695-
next
706+
peek_node = node_stack[-1]
707+
if peek_node.children? && !visited[-1]
708+
visited[-1] = true
709+
peek_node.send(:children_array).reverse_each do |child|
710+
next unless child
711+
712+
node_stack << child
713+
visited << false
714+
end
696715
else
697-
yield node_stack.shift.node # Pop and yield the current node
716+
yield node_stack.pop
717+
visited.pop
698718
end
699719
end
700720

@@ -727,7 +747,7 @@ def breadth_each
727747

728748
yield node_to_traverse
729749
# Enqueue the children from left to right.
730-
node_to_traverse.children.each do |child|
750+
node_to_traverse.send(:children_array).each do |child|
731751
node_queue << child if child
732752
end
733753
end
@@ -756,6 +776,12 @@ def children(&block)
756776
end
757777
end
758778

779+
def children_array
780+
@children
781+
end
782+
783+
private :children_array
784+
759785
# Yields every leaf node of the (sub)tree rooted at this node to the
760786
# specified block.
761787
#
@@ -793,7 +819,7 @@ def each_level
793819
level = [self]
794820
until level.empty?
795821
yield level
796-
level = level.flat_map(&:children).filter_map { |child| child }
822+
level = level.flat_map { |node| node.send(:children_array) }.filter_map { |child| child }
797823
end
798824
self
799825
else
@@ -810,15 +836,15 @@ def each_level
810836
#
811837
# @return [Tree::TreeNode] The first child, or +nil+ if none is present.
812838
def first_child
813-
@children.first
839+
children_array.first
814840
end
815841

816842
# Last child of this node.
817843
# Will be +nil+ if no children are present.
818844
#
819845
# @return [Tree::TreeNode] The last child, or +nil+ if none is present.
820846
def last_child
821-
@children.last
847+
children_array.last
822848
end
823849

824850
# @!group Navigating the Sibling Nodes
@@ -836,7 +862,7 @@ def last_child
836862
# @see #first_sibling?
837863
# @see #last_sibling
838864
def first_sibling
839-
root? ? self : parent.children.first
865+
root? ? self : parent.send(:children_array).first
840866
end
841867

842868
# Returns +true+ if this node is the first sibling at its level.
@@ -864,7 +890,7 @@ def first_sibling?
864890
# @see #last_sibling?
865891
# @see #first_sibling
866892
def last_sibling
867-
root? ? self : parent.children.last
893+
root? ? self : parent.send(:children_array).last
868894
end
869895

870896
# Returns +true+ if this node is the last sibling at its level.
@@ -897,7 +923,9 @@ def last_sibling?
897923
# @see #last_sibling
898924
def siblings
899925
if block_given?
900-
parent.children.each do |sibling|
926+
return self if root?
927+
928+
parent.send(:children_array).each do |sibling|
901929
next unless sibling
902930
next if sibling == self
903931

@@ -908,7 +936,7 @@ def siblings
908936
return [] if root?
909937

910938
siblings = []
911-
parent.children do |my_sibling|
939+
parent.send(:children_array).each do |my_sibling|
912940
next unless my_sibling
913941
siblings << my_sibling if my_sibling != self
914942
end
@@ -924,7 +952,7 @@ def siblings
924952
#
925953
# @see #siblings
926954
def only_child?
927-
root? ? true : parent.children.size == 1
955+
root? ? true : parent.send(:children_array).count { |child| child } == 1
928956
end
929957

930958
alias is_only_child? only_child? # @todo: Aliased for eventual replacement
@@ -942,8 +970,9 @@ def only_child?
942970
def next_sibling
943971
return nil if root?
944972

945-
idx = parent.children.index(self)
946-
parent.children.at(idx + 1) if idx
973+
siblings = parent.send(:children_array)
974+
idx = siblings.index(self)
975+
siblings.at(idx + 1) if idx
947976
end
948977

949978
# Previous sibling of this node.
@@ -959,8 +988,9 @@ def next_sibling
959988
def previous_sibling
960989
return nil if root?
961990

962-
idx = parent.children.index(self)
963-
parent.children.at(idx - 1) if idx&.positive?
991+
siblings = parent.send(:children_array)
992+
idx = siblings.index(self)
993+
siblings.at(idx - 1) if idx&.positive?
964994
end
965995

966996
# @!endgroup

lib/tree/binarytree.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ def set_child_at(child, at_index)
226226
else
227227
@children[at_index] = nil
228228
end
229+
send(:invalidate_size_cache_upwards!)
229230
child
230231
end
231232

lib/tree/utils/metrics_methods.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ module TreeMetricsHandler
5151
# Size:: Total number nodes in the subtree including this node.
5252
#
5353
# @return [Integer] Total number of nodes in this (sub)tree.
54-
def size
55-
count
56-
end
54+
def size
55+
@node_size ||= count
56+
end
5757

5858
# @!attribute [r] length
5959
# Convenience synonym for {#size}.

test/benchmark_tree.rb

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# benchmark_tree.rb - This file is part of the RubyTree package.
2+
#
3+
# Copyright (c) 2026 Anupam Sengupta. All rights reserved.
4+
#
5+
# Redistribution and use in source and binary forms, with or without modification,
6+
# are permitted provided that the following conditions are met:
7+
#
8+
# - Redistributions of source code must retain the above copyright notice, this
9+
# list of conditions and the following disclaimer.
10+
#
11+
# - Redistributions in binary form must reproduce the above copyright notice, this
12+
# list of conditions and the following disclaimer in the documentation and/or
13+
# other materials provided with the distribution.
14+
#
15+
# - Neither the name of the organization nor the names of its contributors may
16+
# be used to endorse or promote products derived from this software without
17+
# specific prior written permission.
18+
#
19+
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
20+
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
21+
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
22+
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
23+
# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
24+
# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
25+
# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
26+
# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
27+
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
28+
# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
29+
#
30+
# frozen_string_literal: true
31+
32+
require 'benchmark'
33+
require 'tree'
34+
35+
def build_tree(depth:, breadth:)
36+
root = Tree::TreeNode.new('root')
37+
level = [root]
38+
39+
depth.times do |d|
40+
next_level = []
41+
level.each_with_index do |node, idx|
42+
breadth.times do |b|
43+
child = Tree::TreeNode.new("n#{d}-#{idx}-#{b}")
44+
node << child
45+
next_level << child
46+
end
47+
end
48+
level = next_level
49+
end
50+
51+
root
52+
end
53+
54+
if $PROGRAM_NAME == __FILE__
55+
depth = (ENV['TREE_BENCH_DEPTH'] || 5).to_i
56+
breadth = (ENV['TREE_BENCH_BREADTH'] || 4).to_i
57+
58+
tree = build_tree(depth: depth, breadth: breadth)
59+
60+
Benchmark.bm(28) do |x|
61+
x.report('root lookups') do
62+
50_000.times { tree.children.first.root }
63+
end
64+
65+
x.report('preorder traversal') do
66+
tree.each { |_node| nil }
67+
end
68+
69+
x.report('postorder traversal') do
70+
tree.postordered_each { |_node| nil }
71+
end
72+
73+
x.report('breadth traversal') do
74+
tree.breadth_each { |_node| nil }
75+
end
76+
77+
x.report('size') do
78+
1000.times { tree.size }
79+
end
80+
81+
x.report('to_s') do
82+
1000.times { tree.to_s }
83+
end
84+
end
85+
end

0 commit comments

Comments
 (0)