From 0244da3fd18a49d5b4495f7740207159b4d7bebc Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 17 Feb 2018 07:44:49 +0000 Subject: [PATCH] Rewrite redundant codepatterns introduced by py2 -> py3 conversion The automated python2 -> python3 converter creates some suboptimal code patterns in some cases, notably in its handling of dicts. This commit handles the following cases: * "if x in list(y.keys()):" => "if x in y:" The original code is neuters the O(1) lookup effeciency of a dict by turning it into a list. This occurs a O(n) in converting it to a list and then another O(n) for the lookup. When done in a loop, this becomes O(n * m) rather than the optimal O(m). * "for x in list(y.keys()):" => "for x in y:" OR "for x in list(y):" A dict (y in these cases) operates as an iterator over keys in the dict by default. This makes the entire "list(y.keys())" dance redundant _in most cases_. In a some cases, scour modifies the dict while iterating over it and in those cases, we need a "list(y)" (but not a "y.keys()"). The benefit of this differs between python2 and python3. In python3, we basically "only" avoid function call. In python2, y.keys() generates a list, so here we avoid generating a "throw-away list". The test suite succeed both with "python testscour.py" and "python3 testscour.py" (used 2.7.14+ and 3.6.4 from Debian testing). On a 341kB flame-graph generated by "nytprof" (a perl profiler), this commit changes the runtimes of scour from the range 3.39s - 3.45s to 3.27s - 3.35s making it roughly 3% faster in this case (YMMV, particularly with different input). The timings were recorded using the following command line: time PYTHONPATH=. python3 -m scour.scour --enable-id-stripping \ --shorten-ids --indent=none --enable-comment-stripping -i input.svg -o output.svg This was used 5 times with and 5 times without the patch picking the worst and best time to define the range. The runtime test was only preformed on python3. All changed lines where found with: grep -rE ' in list[(].*[.]keys[(][)][)]:' Signed-off-by: Niels Thykier --- scour/scour.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/scour/scour.py b/scour/scour.py index 58ac7c4..79a9ea8 100644 --- a/scour/scour.py +++ b/scour/scour.py @@ -844,7 +844,7 @@ def unprotected_ids(doc, options): protect_ids_list = options.protect_ids_list.split(",") if options.protect_ids_prefix: protect_ids_prefixes = options.protect_ids_prefix.split(",") - for id in list(identifiedElements.keys()): + for id in list(identifiedElements): protected = False if options.protect_ids_noninkscape and not id[-1].isdigit(): protected = True @@ -868,7 +868,7 @@ def removeUnreferencedIDs(referencedIDs, identifiedElements): global _num_ids_removed keepTags = ['font'] num = 0 - for id in list(identifiedElements.keys()): + for id in identifiedElements: node = identifiedElements[id] if id not in referencedIDs and node.nodeName not in keepTags: node.removeAttribute('id') @@ -1049,7 +1049,7 @@ def moveCommonAttributesToParentGroup(elem, referencedElements): distinctAttrs = [] # loop through all current 'common' attributes - for name in list(commonAttrs.keys()): + for name in commonAttrs: # if this child doesn't match that attribute, schedule it for removal if child.getAttribute(name) != commonAttrs[name]: distinctAttrs.append(name) @@ -1058,7 +1058,7 @@ def moveCommonAttributesToParentGroup(elem, referencedElements): del commonAttrs[name] # commonAttrs now has all the inheritable attributes which are common among all child elements - for name in list(commonAttrs.keys()): + for name in commonAttrs: for child in childElements: child.removeAttribute(name) elem.setAttribute(name, commonAttrs[name]) @@ -1237,7 +1237,7 @@ def removeUnusedAttributesOnParent(elem): for childNum in range(len(childElements)): child = childElements[childNum] inheritedAttrs = [] - for name in list(unusedAttrs.keys()): + for name in unusedAttrs: val = child.getAttribute(name) if val == '' or val is None or val == 'inherit': inheritedAttrs.append(name) @@ -1245,7 +1245,7 @@ def removeUnusedAttributesOnParent(elem): del unusedAttrs[a] # unusedAttrs now has all the parent attributes that are unused - for name in list(unusedAttrs.keys()): + for name in unusedAttrs: elem.removeAttribute(name) num += 1 @@ -1419,7 +1419,7 @@ def removeDuplicateGradients(doc): # get a collection of all elements that are referenced and their referencing elements referencedIDs = findReferencedElements(doc.documentElement) - for masterGrad in list(gradientsToRemove.keys()): + for masterGrad in gradientsToRemove: master_id = masterGrad.getAttribute('id') for dupGrad in gradientsToRemove[masterGrad]: # if the duplicate gradient no longer has a parent that means it was @@ -1599,7 +1599,7 @@ def repairStyle(node, options): # now if any of the properties match known SVG attributes we prefer attributes # over style so emit them and remove them from the style map if options.style_to_xml: - for propName in list(styleMap.keys()): + for propName in list(styleMap): if propName in svgAttributes: node.setAttribute(propName, styleMap[propName]) del styleMap[propName] @@ -1940,7 +1940,7 @@ def removeDefaultAttributeValues(node, options, tainted=set()): attributes = [node.attributes.item(i).nodeName for i in range(node.attributes.length)] for attribute in attributes: if attribute not in tainted: - if attribute in list(default_properties.keys()): + if attribute in default_properties: if node.getAttribute(attribute) == default_properties[attribute]: node.removeAttribute(attribute) num += 1 @@ -1948,9 +1948,9 @@ def removeDefaultAttributeValues(node, options, tainted=set()): tainted = taint(tainted, attribute) # Properties might also occur as styles, remove them too styles = _getStyle(node) - for attribute in list(styles.keys()): + for attribute in list(styles): if attribute not in tainted: - if attribute in list(default_properties.keys()): + if attribute in default_properties: if styles[attribute] == default_properties[attribute]: del styles[attribute] num += 1 @@ -1975,7 +1975,7 @@ def convertColor(value): """ s = value - if s in list(colors.keys()): + if s in colors: s = colors[s] rgbpMatch = rgbp.match(s) @@ -2031,7 +2031,7 @@ def convertColors(element): element.setAttribute(attr, newColorValue) numBytes += (oldBytes - len(element.getAttribute(attr))) # colors might also hide in styles - if attr in list(styles.keys()): + if attr in styles: oldColorValue = styles[attr] newColorValue = convertColor(oldColorValue) oldBytes = len(oldColorValue) @@ -2770,7 +2770,7 @@ def reducePrecision(element): num += len(val) - len(newVal) element.setAttribute(lengthAttr, newVal) # repeat for attributes hidden in styles - if lengthAttr in list(styles.keys()): + if lengthAttr in styles: val = styles[lengthAttr] valLen = SVGLength(val) if valLen.units != Unit.INVALID: