This is an automated email from the ASF dual-hosted git repository. not-in-ldap pushed a commit to branch bschubert/optimize-mapping-node in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit cb34c95009f8a77ea28b7390a5abbbcc428aa171 Author: Benjamin Schubert <[email protected]> AuthorDate: Mon Jun 29 13:55:17 2020 +0000 Small optimizations --- src/buildstream/_yaml.pyx | 6 ++--- src/buildstream/node.pxd | 2 +- src/buildstream/node.pyx | 64 ++++++++++++++++++++++++----------------------- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx index 26c7286..c94b78e 100644 --- a/src/buildstream/_yaml.pyx +++ b/src/buildstream/_yaml.pyx @@ -32,7 +32,7 @@ from ruamel import yaml from ._exceptions import LoadError from .exceptions import LoadErrorReason from . cimport node -from .node cimport MappingNode, ScalarNode, SequenceNode +from .node cimport MappingNode, Node, ScalarNode, SequenceNode # These exceptions are intended to be caught entirely within @@ -188,7 +188,7 @@ cdef class Representer: cdef RepresenterState _handle_wait_value_MappingStartEvent(self, object ev): cdef RepresenterState new_state = self._handle_doc_MappingStartEvent(ev) key = self.keys.pop() - (<MappingNode> self.output[-2])._set(key, self.output[-1]) + (<MappingNode> self.output[-2])._set(key, <Node> self.output[-1]) return new_state cdef RepresenterState _handle_wait_key_MappingEndEvent(self, object ev): @@ -206,7 +206,7 @@ cdef class Representer: cdef RepresenterState _handle_wait_value_SequenceStartEvent(self, object ev): self.output.append(SequenceNode.__new__( SequenceNode, self._file_index, ev.start_mark.line, ev.start_mark.column, [])) - (<MappingNode> self.output[-2])._set(self.keys[-1], self.output[-1]) + (<MappingNode> self.output[-2])._set(self.keys[-1], <Node> self.output[-1]) return RepresenterState.wait_list_item cdef RepresenterState _handle_wait_list_item_SequenceStartEvent(self, object ev): diff --git a/src/buildstream/node.pxd b/src/buildstream/node.pxd index 769de9a..ca773b4 100644 --- a/src/buildstream/node.pxd +++ b/src/buildstream/node.pxd @@ -71,7 +71,7 @@ cdef class MappingNode(Node): # Private Methods cdef void __composite(self, MappingNode target, list path) except * - cdef void _set(self, str key, object value) + cdef void _set(self, str key, Node value) cdef Node _get(self, str key, default, default_constructor) diff --git a/src/buildstream/node.pyx b/src/buildstream/node.pyx index 42a0794..406bbce 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) #
