Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent isempty(children(node)) from calling iterate (20% improvement in iteration speed) #122

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Get the number of leaves in the tree rooted at `node`. Leaf nodes have a breadth
By default this recurses through all nodes in the tree and so may be slow if a more specialized
method has not been implemented for the given type.
"""
treebreadth(node) = isempty(children(node)) ? 1 : mapreduce(treebreadth, +, children(node))
treebreadth(node) = ischildenempty(children(node)) ? 1 : mapreduce(treebreadth, +, children(node))


"""
Expand All @@ -163,7 +163,7 @@ Get the maximum depth from `node` to any of its descendants. Leaf nodes have a h
By default this recurses through all nodes in the tree and so may be slow if a more specialized
method has not been implemented for the given type.
"""
treeheight(node) = isempty(children(node)) ? 0 : 1 + mapreduce(treeheight, max, children(node))
treeheight(node) = ischildenempty(children(node)) ? 0 : 1 + mapreduce(treeheight, max, children(node))

"""
descendleft(node)
Expand All @@ -173,7 +173,7 @@ Descend from the node `node` to the first encountered leaf node by recursively c
"""
function descendleft(node)
ch = children(node)
isempty(ch) && return node
ischildenempty(ch) && return node
descendleft(first(ch))
end

Expand Down Expand Up @@ -273,3 +273,15 @@ function StableNode{T}(𝒻, node) where {T}
StableNode{T}(convert(T, 𝒻(node)), map(n -> StableNode{T}(𝒻, n), children(node)))
end

# isempty check for children that is consistent with interface
# without this the fallback isempty can waste a call to iterate
# tested the test (ch===()) but it ended up being slopwer
ischildenempty(@nospecialize ch::Tuple) = (@inline; ch == ())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably be ischildrenempty... but this is not correct English. Also the @inline; is broken syntax. It may be better to just manually insert this elision where it would benefit than add an interface for it.


#fallback definition
ischildenempty(ch) = (@inline; isempty(ch))





4 changes: 4 additions & 0 deletions src/cursors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -374,3 +374,7 @@ TreeCursor(::NodeTypeUnknown, ::NonIndexedChildren, ::ParentLinks, ::ImplicitSib
# extra methods to resolve ambiguity
TreeCursor(::NodeTypeUnknown, ::IndexedChildren, ::StoredParents, ::StoredSiblings, node) = TrivialCursor(node)
TreeCursor(::NodeTypeUnknown, ::NonIndexedChildren, ::StoredParents, ::StoredSiblings, node) = TrivialCursor(node)

function ischildenempty(x::TreeCursor)
(ischildenempty ∘ children ∘ nodevalue)(x)
end
4 changes: 2 additions & 2 deletions src/iteration.jl
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ initial(::Type{PreOrderState}, node) = PreOrderState(node)
function next(f, s::PreOrderState)
if f(nodevalue(s.cursor))
ch = children(s.cursor)
isempty(ch) || return instance(PreOrderState, first(ch))
ischildenempty(ch) || return instance(PreOrderState, first(ch))
end

csr = s.cursor
Expand Down Expand Up @@ -376,7 +376,7 @@ Base.iterate(ti::StatelessBFS) = (ti.root, [])
function _descend_left(inds, next_node, level)
while length(inds) ≠ level
ch = children(next_node)
isempty(ch) && break
ischildenempty(ch) && break
push!(inds, 1)
next_node = first(ch)
end
Expand Down
2 changes: 1 addition & 1 deletion src/printing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ function print_tree(printnode::Function, io::IO, node;
c = children(node)

# No children?
isempty(c) && return
ischildenempty(c) && return

# Reached max depth?
if depth ≥ maxdepth
Expand Down