Merge pull request #93 from VEZY/bulk-node-creation-optimisations

Optimize bulk node creation

Author: VEZY

src/compute_MTG/caching.jl

@@ -57,16 +57,19 @@ function cache_nodes!(node; scale=nothing, symbol=nothing, link=nothing, filter_
     symbol = normalize_symbol_filter(symbol)
     link = normalize_link_filter(link)
     # The cache is already present:
-    if length(node_traversal_cache(node)) != 0 && haskey(node_traversal_cache(node), cache_name(scale, symbol, link, all, filter_fun))
+    cache_key = cache_name(scale, symbol, link, all, filter_fun)
+    cache = _maybe_traversal_cache(node)
+    if cache !== nothing && !isempty(cache) && haskey(cache, cache_key)
         if !overwrite
             error("The node already has a cache for this combination of filters. Hint: use `overwrite=true` if needed.")
         else
             # We have to delete the cache first because else it would be used in the traversal below:
-            delete!(node_traversal_cache(node), cache_name(scale, symbol, link, all, filter_fun))
+            delete!(cache, cache_key)
         end
     end
 
-    node_traversal_cache(node)[cache_name(scale, symbol, link, all, filter_fun)] = traverse(
+    cache = node_traversal_cache(node)
+    cache[cache_key] = traverse(
         node,
         node -> node,
         scale=scale, symbol=symbol, link=link, filter_fun=filter_fun, all=all

src/compute_MTG/traverse.jl

@@ -113,8 +113,8 @@ function traverse!(node::Node, f::Function, args...; scale=nothing, symbol=nothi
     link = normalize_link_filter(link)
 
     # If the node has already a cache of the traversal, we use it instead of traversing the mtg:
-    cache = node_traversal_cache(node)
-    if !isempty(cache)
+    cache = _maybe_traversal_cache(node)
+    if cache !== nothing && !isempty(cache)
         cache_key = cache_name(scale, symbol, link, all, filter_fun)
         cached_nodes = get(cache, cache_key, nothing)
         if cached_nodes !== nothing
@@ -172,8 +172,8 @@ function traverse(node::Node, f::Function, args...; scale=nothing, symbol=nothin
     # NB: f has to return someting here, if its a mutating function, use traverse!
 
     # If the node has already a cache of the traversal, we use it instead of traversing the mtg:
-    cache = node_traversal_cache(node)
-    if !isempty(cache)
+    cache = _maybe_traversal_cache(node)
+    if cache !== nothing && !isempty(cache)
         cache_key = cache_name(scale, symbol, link, all, filter_fun)
         cached_nodes = get(cache, cache_key, nothing)
 

src/read_MTG/parse_mtg.jl

@@ -71,14 +71,15 @@ function parse_mtg!(f, classes, features, line, l, mtg_type)
     node_id = [1]
 
     tree_dict = Dict{Int,Node}()
+    feature_names = isnothing(features) ? nothing : Symbol.(features.NAME)
 
     # for i in Iterators.drop(eachindex(splitted_MTG), 1)
     # node_attributes(tree_dict[4])
     try
         while !eof(f)
             l[1] = next_line!(f, line; whitespace=false)
             length(l[1]) == 0 && continue # ignore empty line
-            parse_line_to_node!(tree_dict, l, line, attr_column_start, last_node_column, node_id, mtg_type, features, classes)
+            parse_line_to_node!(tree_dict, l, line, attr_column_start, last_node_column, node_id, mtg_type, features, feature_names, classes)
         end
     catch e
         error(
@@ -142,7 +143,7 @@ Parse MTG node attributes names, values and type
 A list of attributes
 
 """
-function parse_MTG_node_attr(node_data, features, attr_column_start, line; force=false)
+function parse_MTG_node_attr(node_data, features, feature_names, attr_column_start, line; force=false)
 
     if length(node_data) < attr_column_start
         return init_empty_attr()
@@ -155,75 +156,64 @@ function parse_MTG_node_attr(node_data, features, attr_column_start, line; force
             ". Please check line ", line, " of the MTG:\n", join(node_data, "\t"))
     end
 
-    node_attr = Dict{String,Any}(zip(features.NAME[eachindex(node_data_attr)],
-        fill(missing, length(node_data_attr))))
+    node_attr = Dict{Symbol,Any}()
+    sizehint!(node_attr, length(node_data_attr))
 
     node_type = features.TYPE
 
     # node_data_attr is always read in order so names and types correspond to values in features
     for i in eachindex(node_data_attr)
+        feature_name = feature_names[i]
         if node_data_attr[i] == "" || node_data_attr[i] == "NA"
-            pop!(node_attr, features.NAME[i])
             continue
         end
 
         if node_type[i] == "INT"
             try
-                node_attr[features.NAME[i]] = parse(Int, node_data_attr[i])
+                node_attr[feature_name] = parse(Int, node_data_attr[i])
             catch e
                 if !force
                     error("Found issue in the MTG when converting column $(features[i,1]) ",
                         "with value $(node_data_attr[i]) into Integer.",
                         " Please check line ", line, " of the MTG:\n", join(node_data, "\t"))
                 end
-                pop!(node_attr, features.NAME[i])
             end
         elseif node_type[i] == "BOOLEAN"
             try
-                node_attr[features.NAME[i]] = parse(Bool, node_data_attr[i])
+                node_attr[feature_name] = parse(Bool, node_data_attr[i])
             catch e
                 if !force
                     error("Found issue in the MTG when converting column $(features[i,1]) ",
                         "with value $(node_data_attr[i]) into Boolean.",
                         " Please check line ", line, " of the MTG:\n", join(node_data, "\t"))
                 end
-                pop!(node_attr, features.NAME[i])
             end
         elseif node_type[i] == "DD/MM/YY"
             try
-                node_attr[features.NAME[i]] = Date(node_data_attr[i], dateformat"d/m/y")
+                node_attr[feature_name] = Date(node_data_attr[i], dateformat"d/m/y")
             catch e
                 if !force
                     error("Found issue in the MTG when converting column $(features[i,1]) ",
                         "with value $(node_data_attr[i]) into a date with format 'day/month/year'.",
                         " Please check line ", line, " of the MTG:\n", join(node_data, "\t"))
                 end
-                pop!(node_attr, features.NAME[i])
             end
-        elseif node_type[i] == "REAL" || (node_type[i] == "ALPHA" && in(features.NAME[i], ("Width", "Length")))
+        elseif node_type[i] == "REAL" || (node_type[i] == "ALPHA" && feature_name in (:Width, :Length))
             try
-                node_attr[features.NAME[i]] = parse(Float64, node_data_attr[i])
+                node_attr[feature_name] = parse(Float64, node_data_attr[i])
             catch e
                 if !force
                     error("Found issue in the MTG when converting column $(features[i,1]) ",
                         "with value $(node_data_attr[i]) into Floating point number.",
                         " Please check line ", line, " of the MTG:\n", join(node_data, "\t"))
                 end
-                pop!(node_attr, features.NAME[i])
             end
         else
-            node_attr[features.NAME[i]] = node_data_attr[i]
+            node_attr[feature_name] = node_data_attr[i]
         end
     end
 
-    parse_node_attributes(node_attr)
-end
-
-"""
-Instantiate a `ColumnarAttrs` struct with `node_attr` keys and values.
-"""
-function parse_node_attributes(node_attr)
-    ColumnarAttrs(Dict{Symbol,Any}(zip(Symbol.(keys(node_attr)), values(node_attr))))
+    ColumnarAttrs(node_attr)
 end
 
 init_empty_attr() = ColumnarAttrs()
@@ -235,7 +225,7 @@ init_empty_attr() = ColumnarAttrs()
 Parse a line of the MTG file to a node and add it to the tree dictionary.
 It may also add several nodes if the line contains several MTG elements.
 """
-function parse_line_to_node!(tree_dict, l, line, attr_column_start, last_node_column, node_id, mtg_type, features, classes)
+function parse_line_to_node!(tree_dict, l, line, attr_column_start, last_node_column, node_id, mtg_type, features, feature_names, classes)
 
     splitted_MTG = split(l[1], "\t")
     node_column = findfirst(x -> length(x) > 0, splitted_MTG)
@@ -259,7 +249,7 @@ function parse_line_to_node!(tree_dict, l, line, attr_column_start, last_node_co
 
     # Get node attributes:
     if features !== nothing
-        node_attr = parse_MTG_node_attr(node_data, features, node_attr_column_start, line)
+        node_attr = parse_MTG_node_attr(node_data, features, feature_names, node_attr_column_start, line)
     else
         # if there are no attribute in the MTG, we create an empty attribute:
         node_attr = init_empty_attr()

src/types/Attributes.jl

@@ -641,13 +641,7 @@ function Base.get(attrs::ColumnarAttrs, key::Symbol, default::T) where {T}
 end
 Base.get(attrs::ColumnarAttrs, key, default) = get(attrs, _normalize_attr_key(key), default)
 
-function Base.setindex!(attrs::ColumnarAttrs, value::T, key::Symbol) where {T}
-    if !_isbound(attrs)
-        attrs.staged[key] = value
-        return value
-    end
-    store, bid, row = _bound_store_bid_row(attrs.ref)
-    bucket = store.buckets[bid]
+@inline function _set_value_bound!(bucket::SymbolBucket, row::Int, key::Symbol, value::T) where {T}
     col_idx = get(bucket.col_index, key, 0)
     if col_idx == 0
         _set_value!(bucket, row, key, value)
@@ -666,6 +660,17 @@ function Base.setindex!(attrs::ColumnarAttrs, value::T, key::Symbol) where {T}
 
     return value
 end
+
+function Base.setindex!(attrs::ColumnarAttrs, value::T, key::Symbol) where {T}
+    if !_isbound(attrs)
+        attrs.staged[key] = value
+        return value
+    end
+    store, bid, row = _bound_store_bid_row(attrs.ref)
+    bucket = store.buckets[bid]
+    _set_value_bound!(bucket, row, key, value)
+    return value
+end
 Base.setindex!(attrs::ColumnarAttrs, value, key) = setindex!(attrs, value, _normalize_attr_key(key))
 
 function Base.iterate(attrs::ColumnarAttrs, state=nothing)

src/types/Node.jl

@@ -55,7 +55,7 @@ mutable struct Node{N<:AbstractNodeMTG,A}
     "Node attributes. Can be anything really"
     attributes::A
     "Cache for mtg nodes traversal"
-    traversal_cache::Dict{String,Vector{Node{N,A}}}
+    traversal_cache::Union{Nothing,Dict{String,Vector{Node{N,A}}}}
 end
 
 # All deprecated methods (the ones with a node name) :
@@ -68,7 +68,7 @@ end
 
 function Node(id::Int, MTG::T, attributes::ColumnarAttrs) where {T<:AbstractNodeMTG}
     node = Node{T,ColumnarAttrs}(
-        id, nothing, Vector{Node{T,ColumnarAttrs}}(), MTG, attributes, Dict{String,Vector{Node{T,ColumnarAttrs}}}()
+        id, nothing, Vector{Node{T,ColumnarAttrs}}(), MTG, attributes, nothing
     )
     init_columnar_root!(attributes, id, getfield(MTG, :symbol))
     return node
@@ -101,7 +101,7 @@ end
 
 function Node(id::Int, parent::Node{M,ColumnarAttrs}, MTG::M, attributes::ColumnarAttrs) where {M<:AbstractNodeMTG}
     node = Node{M,ColumnarAttrs}(
-        id, parent, Vector{Node{M,ColumnarAttrs}}(), MTG, attributes, Dict{String,Vector{Node{M,ColumnarAttrs}}}()
+        id, parent, Vector{Node{M,ColumnarAttrs}}(), MTG, attributes, nothing
     )
     addchild!(parent, node)
     bind_columnar_child!(node_attributes(parent), attributes, id, getfield(MTG, :symbol))
@@ -517,7 +517,16 @@ Base.names(mtg::T) where {T<:MultiScaleTreeGraph.Node} = get_attributes(mtg)
 
 Get the traversal cache of the node if any.
 """
-node_traversal_cache(node::Node) = getfield(node, :traversal_cache)
+@inline _maybe_traversal_cache(node::Node) = getfield(node, :traversal_cache)
+
+function node_traversal_cache(node::Node{T,A}) where {T,A}
+    cache = getfield(node, :traversal_cache)
+    if cache === nothing
+        cache = Dict{String,Vector{Node{T,A}}}()
+        setfield!(node, :traversal_cache, cache)
+    end
+    return cache
+end
 
 Base.getproperty(node::Node, key::Symbol) = unsafe_getindex(node, key)
 Base.hasproperty(node::Node, key::Symbol) = haskey(node_attributes(node), key)