Small optimizationsbschubert/optimize-mapping-node

1 files changed, 33 insertions, 31 deletions

diff --git a/src/buildstream/node.pyx b/src/buildstream/node.pyx
index 42a0794c8..406bbce1b 100644
--- a/src/buildstream/node.pyx
+++ b/src/buildstream/node.pyx
@@ -504,7 +504,37 @@ cdef class MappingNode(Node):
         del self.value[key]
 
     def __setitem__(self, str key, object value):
-        self._set(key, value)
+        cdef Node old_value
+        cdef Node node
+
+        if type(value) not in [MappingNode, ScalarNode, SequenceNode]:
+            node = __create_node_recursive(value, self)
+
+            # FIXME: Do we really want to override provenance?
+            #
+            # Related to https://gitlab.com/BuildStream/buildstream/issues/1058
+            #
+            # There are only two cases were nodes are set in the code (hence without provenance):
+            #   - When automatic variables are set by the core (e-g: max-jobs)
+            #   - when plugins call Element.set_public_data
+            #
+            # The first case should never throw errors, so it is of limited interests.
+            #
+            # The second is more important. What should probably be done here is to have 'set_public_data'
+            # able of creating a fake provenance with the name of the plugin, the project and probably the
+            # element name.
+            #
+            # We would therefore have much better error messages, and would be able to get rid of most synthetic
+            # nodes.
+            old_value = <Node> self.value.get(key)
+            if old_value:
+                node.file_index = old_value.file_index
+                node.line = old_value.line
+                node.column = old_value.column
+        else:
+            node = value
+
+        self._set(key, node)
 
     #############################################################
     #                       Public Methods                      #
@@ -1108,38 +1138,10 @@ cdef class MappingNode(Node):
     #   key (str): the key for which to set the value
     #   value (object): the value to set
     #
-    cdef void _set(self, str key, object value):
+    cdef void _set(self, str key, Node value):
         cdef Node old_value
         cdef intern_key = sys.intern(key)
-
-        if type(value) in [MappingNode, ScalarNode, SequenceNode]:
-            self.value[intern_key] = value
-        else:
-            node = __create_node_recursive(value, self)
-
-            # FIXME: Do we really want to override provenance?
-            #
-            # Related to https://gitlab.com/BuildStream/buildstream/issues/1058
-            #
-            # There are only two cases were nodes are set in the code (hence without provenance):
-            #   - When automatic variables are set by the core (e-g: max-jobs)
-            #   - when plugins call Element.set_public_data
-            #
-            # The first case should never throw errors, so it is of limited interests.
-            #
-            # The second is more important. What should probably be done here is to have 'set_public_data'
-            # able of creating a fake provenance with the name of the plugin, the project and probably the
-            # element name.
-            #
-            # We would therefore have much better error messages, and would be able to get rid of most synthetic
-            # nodes.
-            old_value = self.value.get(intern_key)
-            if old_value:
-                node.file_index = old_value.file_index
-                node.line = old_value.line
-                node.column = old_value.column
-
-            self.value[intern_key] = node
+        self.value[intern_key] = value
 
     # _get(key, default, default_constructor)
     #