[Notes] [Git][BuildStream/buildstream][danielsilverstone-ct/further-optimisations] 7 commits: plugin.py: Redirect DEBUG from `__del__` to `sys.stderr`

[Daniel Silverstone <gitlab mg gitlab com>]

Title: GitLab

Daniel Silverstone pushed to branch danielsilverstone-ct/further-optimisations at BuildStream / buildstream

Commits:

• 7738f6df
  by Daniel Silverstone at 2018-11-09T11:25:44Z

  plugin.py: Redirect DEBUG from `__del__` to `sys.stderr`
  
  Previously, debug messages from `Plugin.__del__()` went to stdout
  which was a problem for the test suite in extremely rare and hard to
  predict circumstances.  This corrects that by sending the message to
  stderr where all the rest of the messages tend to go.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• 62f59eaa
  by Valentin David at 2018-11-09T13:30:06Z

  Merge branch 'danielsilverstone-ct/plugin-destroy-debug-to-stderr' into 'master'
  
  plugin.py: Redirect DEBUG from `__del__` to `sys.stderr`
  
  See merge request BuildStream/buildstream!944

• 4eeb2b8c
  by Daniel Silverstone at 2018-11-09T13:49:42Z

  _yaml.py: Only retrieve provenance in node_get() when needed
  
  We were indiscriminately retrieving the node's provenance data in the
  `node_get()` function which was accounting for approximately a third of
  the total runtime of `node_get()` which dominates pre-scheduler time in
  `bst build`.  This change ameliorates that situation by only retrieving
  the provenance data when it's actually needed.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• 5d2655a8
  by Daniel Silverstone at 2018-11-09T13:49:42Z

  _yaml.py: Reduce use of `isinstance()` in `node_sanitize()`
  
  We know that nodes are typically one of:
  str, list, dict, bool, tuple, NoneType or our ChainMap
  Of these, dict and ChainMap are Mapping, only list is list
  and the rest are returned unchanged.  We can reduce/defer our use
  of isinstance here, dramatically, improving performance.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• f924811a
  by Daniel Silverstone at 2018-11-09T13:49:42Z

  _cachekey.py: Add a variant of the calculator which take pre-sanitized input
  
  In order to support a case where the caller already has pre-sanitized input
  this variant of the cache key generator needs to exist.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• fa9c3c6c
  by Daniel Silverstone at 2018-11-09T13:49:42Z

  element.py: Alter Element.__calculate_cache_key() to pre-santize inputs
  
  In order to reduce the effort we spend in `_yaml.node_sanitize()` take
  advantage of how `OrderedDict` works and pre-sanitize the majority of
  the cache-key once.  This approximately halves the amount of effort we
  spend in `_yaml.node_sanitize()` in pre-scheduler build scenarios.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

• 6e08d996
  by Daniel Silverstone at 2018-11-09T13:49:42Z

  _yaml.py: Remove use of isinstance() in `{node,list}_{chain_,}copy`
  
  A non-trivial proportion of time pre-scheduler in `bst build` is spent
  copying (or chain-copying) nodes.  Approximately a quarter of the time
  spent in that effort is in `isinstance()`.  This removes that CPU load.
  
  Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>
  

4 changed files:

• buildstream/_cachekey.py
• buildstream/_yaml.py
• buildstream/element.py
• buildstream/plugin.py

Changes:

buildstream/_cachekey.py

---

...	...	@@ -40,3 +40,20 @@ def generate_key(value):
40	40	ordered = _yaml.node_sanitize(value)
41	41	string = pickle.dumps(ordered)
42	42	return hashlib.sha256(string).hexdigest()
	43	+
	44	+
	45	+# generate_key_pre_sanitized()
	46	+#
	47	+# Generate an sha256 hex digest from the given value. The value
	48	+# must be (a) compatible with generate_key() and (b) already have
	49	+# been passed through _yaml.node_sanitize()
	50	+#
	51	+# Args:
	52	+# value: A sanitized value to get a key for
	53	+#
	54	+# Returns:
	55	+# (str): An sha256 hex digest of the given value
	56	+#
	57	+def generate_key_pre_sanitized(value):
	58	+ string = pickle.dumps(value)
	59	+ return hashlib.sha256(string).hexdigest()

buildstream/_yaml.py

---

...	...	@@ -363,8 +363,8 @@ _sentinel = object()
363	363	#
364	364	def node_get(node, expected_type, key, indices=None, default_value=_sentinel):
365	365	value = node.get(key, default_value)
366		- provenance = node_get_provenance(node)
367	366	if value is _sentinel:
	367	+ provenance = node_get_provenance(node)
368	368	raise LoadError(LoadErrorReason.INVALID_DATA,
369	369	"{}: Dictionary did not contain expected key '{}'".format(provenance, key))
370	370
...	...	@@ -914,9 +914,20 @@ RoundTripRepresenter.add_representer(SanitizedDict,
914	914	# Only dicts are ordered, list elements are left in order.
915	915	#
916	916	def node_sanitize(node):
	917	+ # Short-circuit None which occurs ca. twice per element
	918	+ if node is None:
	919	+ return node
	920	+
	921	+ node_type = type(node)
	922	+ # Next short-circuit integers, floats, strings, booleans, and tuples
	923	+ if node_type in (int, float, str, bool, tuple):
	924	+ return node
	925	+ # Now short-circuit lists
	926	+ elif node_type is list:
	927	+ return [node_sanitize(elt) for elt in node]
917	928
918		- if isinstance(node, collections.Mapping):
919		-
	929	+ # Finally ChainMap and dict, and other Mappings need special handling
	930	+ if node_type in (dict, ChainMap) or isinstance(node, collections.Mapping):
920	931	result = SanitizedDict()
921	932
922	933	key_list = [key for key, _ in node_items(node)]
...	...	@@ -924,10 +935,10 @@ def node_sanitize(node):
924	935	result[key] = node_sanitize(node[key])
925	936
926	937	return result
927		-
928	938	elif isinstance(node, list):
929	939	return [node_sanitize(elt) for elt in node]
930	940
	941	+ # Everything else (such as commented scalars) just gets returned as-is.
931	942	return node
932	943
933	944
...	...	@@ -1055,16 +1066,48 @@ class ChainMap(collections.ChainMap):
1055	1066	except KeyError:
1056	1067	return default
1057	1068
	1069	+# Node copying
	1070	+#
	1071	+# Unfortunately we copy nodes a *lot* and `isinstance()` is super-slow when
	1072	+# things from collections.abc get involved. The result is the following
	1073	+# intricate but substantially faster group of tuples and the use of `in`.
	1074	+#
	1075	+# If any of the {node,list}_{chain_,}_copy routines raise a ValueError
	1076	+# then it's likely additional types need adding to these tuples.
	1077	+
	1078	+# When chaining a copy, these types are skipped since the ChainMap will
	1079	+# retrieve them from the source node when needed. Other copiers might copy
	1080	+# them, so we call them __quick_types.
	1081	+__quick_types = (str, bool,
	1082	+ yaml.scalarstring.PreservedScalarString,
	1083	+ yaml.scalarstring.SingleQuotedScalarString,
	1084	+ yaml.scalarstring.DoubleQuotedScalarString)
	1085	+
	1086	+# These types have to be iterated like a dictionary
	1087	+__dict_types = (dict, ChainMap, yaml.comments.CommentedMap)
	1088	+
	1089	+# These types have to be iterated like a list
	1090	+__list_types = (list, yaml.comments.CommentedSeq)
	1091	+
	1092	+# These are the provenance types, which have to be cloned rather than any other
	1093	+# copying tactic.
	1094	+__provenance_types = (Provenance, DictProvenance, MemberProvenance, ElementProvenance)
	1095	+
1058	1096
1059	1097	def node_chain_copy(source):
1060	1098	copy = ChainMap({}, source)
1061	1099	for key, value in source.items():
1062		- if isinstance(value, collections.Mapping):
	1100	+ value_type = type(value)
	1101	+ if value_type in __dict_types:
1063	1102	copy[key] = node_chain_copy(value)
1064		- elif isinstance(value, list):
	1103	+ elif value_type in __list_types:
1065	1104	copy[key] = list_chain_copy(value)
1066		- elif isinstance(value, Provenance):
	1105	+ elif value_type in __provenance_types:
1067	1106	copy[key] = value.clone()
	1107	+ elif value_type in __quick_types:
	1108	+ pass # No need to copy these, the chainmap deals with it
	1109	+ else:
	1110	+ raise ValueError("Unable to be quick about node_chain_copy of {}".format(value_type))
1068	1111
1069	1112	return copy
1070	1113
...	...	@@ -1072,14 +1115,17 @@ def node_chain_copy(source):
1072	1115	def list_chain_copy(source):
1073	1116	copy = []
1074	1117	for item in source:
1075		- if isinstance(item, collections.Mapping):
	1118	+ item_type = type(item)
	1119	+ if item_type in __dict_types:
1076	1120	copy.append(node_chain_copy(item))
1077		- elif isinstance(item, list):
	1121	+ elif item_type in __list_types:
1078	1122	copy.append(list_chain_copy(item))
1079		- elif isinstance(item, Provenance):
	1123	+ elif item_type in __provenance_types:
1080	1124	copy.append(item.clone())
1081		- else:
	1125	+ elif item_type in __quick_types:
1082	1126	copy.append(item)
	1127	+ else: # Fallback
	1128	+ raise ValueError("Unable to be quick about list_chain_copy of {}".format(item_type))
1083	1129
1084	1130	return copy
1085	1131
...	...	@@ -1087,14 +1133,17 @@ def list_chain_copy(source):
1087	1133	def node_copy(source):
1088	1134	copy = {}
1089	1135	for key, value in source.items():
1090		- if isinstance(value, collections.Mapping):
	1136	+ value_type = type(value)
	1137	+ if value_type in __dict_types:
1091	1138	copy[key] = node_copy(value)
1092		- elif isinstance(value, list):
	1139	+ elif value_type in __list_types:
1093	1140	copy[key] = list_copy(value)
1094		- elif isinstance(value, Provenance):
	1141	+ elif value_type in __provenance_types:
1095	1142	copy[key] = value.clone()
1096		- else:
	1143	+ elif value_type in __quick_types:
1097	1144	copy[key] = value
	1145	+ else:
	1146	+ raise ValueError("Unable to be quick about node_copy of {}".format(value_type))
1098	1147
1099	1148	ensure_provenance(copy)
1100	1149
...	...	@@ -1104,14 +1153,17 @@ def node_copy(source):
1104	1153	def list_copy(source):
1105	1154	copy = []
1106	1155	for item in source:
1107		- if isinstance(item, collections.Mapping):
	1156	+ item_type = type(item)
	1157	+ if item_type in __dict_types:
1108	1158	copy.append(node_copy(item))
1109		- elif isinstance(item, list):
	1159	+ elif item_type in __list_types:
1110	1160	copy.append(list_copy(item))
1111		- elif isinstance(item, Provenance):
	1161	+ elif item_type in __provenance_types:
1112	1162	copy.append(item.clone())
1113		- else:
	1163	+ elif item_type in __quick_types:
1114	1164	copy.append(item)
	1165	+ else:
	1166	+ raise ValueError("Unable to be quick about list_copy of {}".format(item_type))
1115	1167
1116	1168	return copy
1117	1169

buildstream/element.py

---

...	...	@@ -2053,11 +2053,14 @@ class Element(Plugin):
2053	2053	}
2054	2054
2055	2055	self.__cache_key_dict['fatal-warnings'] = sorted(project._fatal_warnings)
	2056	+ self.__cache_key_dict['dependencies'] = []
	2057	+ self.__cache_key_dict = _yaml.node_sanitize(self.__cache_key_dict)
2056	2058
2057		- cache_key_dict = self.__cache_key_dict.copy()
2058		- cache_key_dict['dependencies'] = dependencies
	2059	+ # This replacement is safe since OrderedDict replaces the value,
	2060	+ # leaving its location in the dictionary alone.
	2061	+ self.__cache_key_dict['dependencies'] = _yaml.node_sanitize(dependencies)
2059	2062
2060		- return _cachekey.generate_key(cache_key_dict)
	2063	+ return _cachekey.generate_key_pre_sanitized(self.__cache_key_dict)
2061	2064
2062	2065	# __can_build_incrementally()
2063	2066	#

buildstream/plugin.py

---

...	...	@@ -111,6 +111,7 @@ Class Reference
111	111
112	112	import os
113	113	import subprocess
	114	+import sys
114	115	from contextlib import contextmanager
115	116	from weakref import WeakValueDictionary
116	117
...	...	@@ -190,7 +191,7 @@ class Plugin():
190	191	# Dont send anything through the Message() pipeline at destruction time,
191	192	# any subsequent lookup of plugin by unique id would raise KeyError.
192	193	if self.__context.log_debug:
193		- print("DEBUG: Destroyed: {}".format(self))
	194	+ sys.stderr.write("DEBUG: Destroyed: {}\n".format(self))
194	195
195	196	def __str__(self):
196	197	return "{kind} {typetag} at {provenance}".format(

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.