_yaml: Remove 'node_copy' and add 'Node.copy()'bschubert/node-copy

Also adaprt every part of the code calling it

7 files changed, 51 insertions, 85 deletions

diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py
index 2831c9765..f74ea85be 100644
--- a/src/buildstream/_includes.py
+++ b/src/buildstream/_includes.py
@@ -71,7 +71,7 @@ class Includes:
                 # Because the included node will be modified, we need
                 # to copy it so that we do not modify the toplevel
                 # node of the provenance.
-                include_node = _yaml.node_copy(include_node)
+                include_node = include_node.copy()
 
                 try:
                     included.add(file_path)
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 424af133e..3d2046035 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -569,7 +569,7 @@ class Project():
             else:
                 raise
 
-        pre_config_node = _yaml.node_copy(self._default_config_node)
+        pre_config_node = self._default_config_node.copy()
         _yaml.composite(pre_config_node, self._project_conf)
 
         # Assert project's format version early, before validating toplevel keys
@@ -612,9 +612,9 @@ class Project():
 
         self._project_includes = Includes(self.loader, copy_tree=False)
 
-        project_conf_first_pass = _yaml.node_copy(self._project_conf)
+        project_conf_first_pass = self._project_conf.copy()
         self._project_includes.process(project_conf_first_pass, only_local=True)
-        config_no_include = _yaml.node_copy(self._default_config_node)
+        config_no_include = self._default_config_node.copy()
         _yaml.composite(config_no_include, project_conf_first_pass)
 
         self._load_pass(config_no_include, self.first_pass_config,
@@ -636,9 +636,9 @@ class Project():
     # Process the second pass of loading the project configuration.
     #
     def _load_second_pass(self):
-        project_conf_second_pass = _yaml.node_copy(self._project_conf)
+        project_conf_second_pass = self._project_conf.copy()
         self._project_includes.process(project_conf_second_pass)
-        config = _yaml.node_copy(self._default_config_node)
+        config = self._default_config_node.copy()
         _yaml.composite(config, project_conf_second_pass)
 
         self._load_pass(config, self.config)
@@ -938,7 +938,7 @@ class Project():
                             .format(plugin_group, expected_groups))
         node_keys = [key for key in _yaml.node_keys(origin)]
         if plugin_group in node_keys:
-            origin_node = _yaml.node_copy(origin)
+            origin_node = origin.copy()
             plugins = origin.get_mapping(plugin_group, default={})
             _yaml.node_set(origin_node, 'plugins', [k for k in _yaml.node_keys(plugins)])
             for group in expected_groups:
diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd
index 8deb17a78..6b0d3fd20 100644
--- a/src/buildstream/_yaml.pxd
+++ b/src/buildstream/_yaml.pxd
@@ -27,6 +27,8 @@ cdef class Node:
     cdef public int line
     cdef public int column
 
+    cpdef Node copy(self)
+
 
 cdef class MappingNode(Node):
     cdef Node get(self, str key, default, default_constructor)
diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx
index 4a49a9832..02e7bf205 100644
--- a/src/buildstream/_yaml.pyx
+++ b/src/buildstream/_yaml.pyx
@@ -74,6 +74,9 @@ cdef class Node:
         # code which has access to such nodes would do this.
         return what in self.value
 
+    cpdef Node copy(self):
+        raise NotImplementedError()
+
 
 cdef class ScalarNode(Node):
 
@@ -85,6 +88,9 @@ cdef class ScalarNode(Node):
         self.line = line
         self.column = column
 
+    cpdef ScalarNode copy(self):
+        return self
+
     cpdef bint is_none(self):
         return self.value is None
 
@@ -129,6 +135,17 @@ cdef class MappingNode(Node):
         self.line = line
         self.column = column
 
+    cpdef MappingNode copy(self):
+        cdef dict copy = {}
+        cdef str key
+        cdef Node value
+
+        for key, value in self.value.items():
+            copy[key] = value.copy()
+
+        return MappingNode(copy, self.file_index, self.line, self.column)
+
+
     cdef Node get(self, str key, object default, object default_constructor):
         value = self.value.get(key, _sentinel)
 
@@ -223,6 +240,15 @@ cdef class SequenceNode(Node):
         self.line = line
         self.column = column
 
+    cpdef SequenceNode copy(self):
+        cdef list copy = []
+        cdef Node entry
+
+        for entry in self.value:
+            copy.append(entry.copy())
+
+        return SequenceNode(copy, self.file_index, self.line, self.column)
+
     cpdef MappingNode mapping_at(self, int index):
         value = self.value[index]
 
@@ -650,7 +676,7 @@ cpdef Node load_data(str data, int file_index=_SYNTHETIC_FILE_INDEX, str file_na
         )
 
     if copy_tree:
-        contents = node_copy(contents)
+        contents = contents.copy()
     return contents
 
 
@@ -1240,73 +1266,11 @@ cpdef void node_validate(Node node, list valid_keys) except *:
                             "{}: Unexpected key: {}".format(provenance, key))
 
 
-# Node copying
-#
-# Unfortunately we copy nodes a *lot* and `isinstance()` is super-slow when
-# things from collections.abc get involved.  The result is the following
-# intricate but substantially faster group of tuples and the use of `in`.
-#
-# If any of the {node,list}_copy routines raise a ValueError
-# then it's likely additional types need adding to these tuples.
-
-
-# These types just have their value copied
-__QUICK_TYPES = (str, bool)
-
 # These are the directives used to compose lists, we need this because it's
 # slightly faster during the node_final_assertions checks
 __NODE_ASSERT_COMPOSITION_DIRECTIVES = ('(>)', '(<)', '(=)')
 
 
-# node_copy()
-#
-# Make a deep copy of the given YAML node, preserving provenance.
-#
-# Args:
-#    source (Node): The YAML node to copy
-#
-# Returns:
-#    (Node): A deep copy of source with provenance preserved.
-#
-cpdef MappingNode node_copy(MappingNode source):
-    cdef dict copy = {}
-    cdef str key
-    cdef Node value
-
-    for key, value in source.value.items():
-        value_type = type(value.value)
-        if value_type is dict:
-            copy[key] = node_copy(value)
-        elif value_type is list:
-            copy[key] = _list_copy(value)
-        elif value_type in __QUICK_TYPES:
-            copy[key] = value
-        else:
-            raise ValueError("Unable to be quick about node_copy of {}".format(value_type))
-
-    return MappingNode(copy, source.file_index, source.line, source.column)
-
-
-# Internal function to help node_copy() but for lists.
-cdef Node _list_copy(Node source):
-    cdef list copy = []
-    cdef Node item
-
-    for item in source.value:
-        item_type = type(item.value)
-
-        if item_type is dict:
-            copy.append(node_copy(item))
-        elif item_type is list:
-            copy.append(_list_copy(item))
-        elif item_type in __QUICK_TYPES:
-            copy.append(item)
-        else:
-            raise ValueError("Unable to be quick about list_copy of {}".format(item_type))
-
-    return SequenceNode(copy, source.file_index, source.line, source.column)
-
-
 # node_final_assertions()
 #
 # This must be called on a fully loaded and composited node,
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 3029c98cd..ab7fd6e9e 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -852,7 +852,7 @@ class Element(Plugin):
 
         data = self.__dynamic_public.get_mapping(domain, default=None)
         if data is not None:
-            data = _yaml.node_copy(data)
+            data = data.copy()
 
         return data
 
@@ -872,7 +872,7 @@ class Element(Plugin):
             self.__load_public_data()
 
         if data is not None:
-            data = _yaml.node_copy(data)
+            data = data.copy()
 
         _yaml.node_set(self.__dynamic_public, domain, data)
 
@@ -1633,7 +1633,7 @@ class Element(Plugin):
 
                 # By default, the dynamic public data is the same as the static public data.
                 # The plugin's assemble() method may modify this, though.
-                self.__dynamic_public = _yaml.node_copy(self.__public)
+                self.__dynamic_public = self.__public.copy()
 
                 # Call the abstract plugin methods
 
@@ -2498,11 +2498,11 @@ class Element(Plugin):
         element_splits = element_bst.get_mapping("split-rules", default={})
 
         if is_junction:
-            splits = _yaml.node_copy(element_splits)
+            splits = element_splits.copy()
         else:
             assert project._splits is not None
 
-            splits = _yaml.node_copy(project._splits)
+            splits = project._splits.copy()
             # Extend project wide split rules with any split rules defined by the element
             _yaml.composite(splits, element_splits)
 
@@ -2552,7 +2552,7 @@ class Element(Plugin):
         if meta.is_junction:
             environment = _yaml.new_empty_node()
         else:
-            environment = _yaml.node_copy(project.base_environment)
+            environment = project.base_environment.copy()
 
         _yaml.composite(environment, default_env)
         _yaml.composite(environment, meta.environment)
@@ -2596,9 +2596,9 @@ class Element(Plugin):
         default_vars = cls.__defaults.get_mapping('variables', default={})
 
         if meta.is_junction:
-            variables = _yaml.node_copy(project.first_pass_config.base_variables)
+            variables = project.first_pass_config.base_variables.copy()
         else:
-            variables = _yaml.node_copy(project.base_variables)
+            variables = project.base_variables.copy()
 
         _yaml.composite(variables, default_vars)
         _yaml.composite(variables, meta.variables)
@@ -2621,7 +2621,7 @@ class Element(Plugin):
 
         # The default config is already composited with the project overrides
         config = cls.__defaults.get_mapping('config', default={})
-        config = _yaml.node_copy(config)
+        config = config.copy()
 
         _yaml.composite(config, meta.config)
         _yaml.node_final_assertions(config)
@@ -2638,7 +2638,7 @@ class Element(Plugin):
                 'build-gid': 0
             })
         else:
-            sandbox_config = _yaml.node_copy(project._sandbox)
+            sandbox_config = project._sandbox.copy()
 
         # Get the platform to ask for host architecture
         platform = Platform.get_platform()
@@ -2647,7 +2647,7 @@ class Element(Plugin):
 
         # The default config is already composited with the project overrides
         sandbox_defaults = cls.__defaults.get_mapping('sandbox', default={})
-        sandbox_defaults = _yaml.node_copy(sandbox_defaults)
+        sandbox_defaults = sandbox_defaults.copy()
 
         _yaml.composite(sandbox_config, sandbox_defaults)
         _yaml.composite(sandbox_config, meta.sandbox)
@@ -2674,12 +2674,12 @@ class Element(Plugin):
     @classmethod
     def __extract_public(cls, meta):
         base_public = cls.__defaults.get_mapping('public', default={})
-        base_public = _yaml.node_copy(base_public)
+        base_public = base_public.copy()
 
         base_bst = base_public.get_mapping('bst', default={})
         base_splits = base_bst.get_mapping('split-rules', default={})
 
-        element_public = _yaml.node_copy(meta.public)
+        element_public = meta.public.copy()
         element_bst = element_public.get_mapping('bst', default={})
         element_splits = element_bst.get_mapping('split-rules', default={})
 
diff --git a/src/buildstream/source.py b/src/buildstream/source.py
index 609836551..6d65811be 100644
--- a/src/buildstream/source.py
+++ b/src/buildstream/source.py
@@ -1286,7 +1286,7 @@ class Source(Plugin):
     @classmethod
     def __extract_config(cls, meta):
         config = cls.__defaults.get_mapping('config', default={})
-        config = _yaml.node_copy(config)
+        config = config.copy()
 
         _yaml.composite(config, meta.config)
         _yaml.node_final_assertions(config)
diff --git a/tests/internals/yaml.py b/tests/internals/yaml.py
index aa83d949a..0df058d62 100644
--- a/tests/internals/yaml.py
+++ b/tests/internals/yaml.py
@@ -183,7 +183,7 @@ def test_composite_preserve_originals(datafiles):
 
     base = _yaml.load(filename)
     overlay = _yaml.load(overlayfile)
-    base_copy = _yaml.node_copy(base)
+    base_copy = base.copy()
     _yaml.composite_dict(base_copy, overlay)
 
     copy_extra = base_copy.get_mapping('extra')