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 <niels@thykier.net>
This commit is contained in:
Niels Thykier 2018-02-17 07:44:49 +00:00 committed by Eduard Braun
parent cb093e9171
commit 2f0b3ea362

View file

@ -846,7 +846,7 @@ def unprotected_ids(doc, options):
protect_ids_list = options.protect_ids_list.split(",") protect_ids_list = options.protect_ids_list.split(",")
if options.protect_ids_prefix: if options.protect_ids_prefix:
protect_ids_prefixes = options.protect_ids_prefix.split(",") protect_ids_prefixes = options.protect_ids_prefix.split(",")
for id in list(identifiedElements.keys()): for id in list(identifiedElements):
protected = False protected = False
if options.protect_ids_noninkscape and not id[-1].isdigit(): if options.protect_ids_noninkscape and not id[-1].isdigit():
protected = True protected = True
@ -870,7 +870,7 @@ def removeUnreferencedIDs(referencedIDs, identifiedElements):
global _num_ids_removed global _num_ids_removed
keepTags = ['font'] keepTags = ['font']
num = 0 num = 0
for id in list(identifiedElements.keys()): for id in identifiedElements:
node = identifiedElements[id] node = identifiedElements[id]
if id not in referencedIDs and node.nodeName not in keepTags: if id not in referencedIDs and node.nodeName not in keepTags:
node.removeAttribute('id') node.removeAttribute('id')
@ -1051,7 +1051,7 @@ def moveCommonAttributesToParentGroup(elem, referencedElements):
distinctAttrs = [] distinctAttrs = []
# loop through all current 'common' attributes # 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 this child doesn't match that attribute, schedule it for removal
if child.getAttribute(name) != commonAttrs[name]: if child.getAttribute(name) != commonAttrs[name]:
distinctAttrs.append(name) distinctAttrs.append(name)
@ -1060,7 +1060,7 @@ def moveCommonAttributesToParentGroup(elem, referencedElements):
del commonAttrs[name] del commonAttrs[name]
# commonAttrs now has all the inheritable attributes which are common among all child elements # 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: for child in childElements:
child.removeAttribute(name) child.removeAttribute(name)
elem.setAttribute(name, commonAttrs[name]) elem.setAttribute(name, commonAttrs[name])
@ -1239,7 +1239,7 @@ def removeUnusedAttributesOnParent(elem):
for childNum in range(len(childElements)): for childNum in range(len(childElements)):
child = childElements[childNum] child = childElements[childNum]
inheritedAttrs = [] inheritedAttrs = []
for name in list(unusedAttrs.keys()): for name in unusedAttrs:
val = child.getAttribute(name) val = child.getAttribute(name)
if val == '' or val is None or val == 'inherit': if val == '' or val is None or val == 'inherit':
inheritedAttrs.append(name) inheritedAttrs.append(name)
@ -1247,7 +1247,7 @@ def removeUnusedAttributesOnParent(elem):
del unusedAttrs[a] del unusedAttrs[a]
# unusedAttrs now has all the parent attributes that are unused # unusedAttrs now has all the parent attributes that are unused
for name in list(unusedAttrs.keys()): for name in unusedAttrs:
elem.removeAttribute(name) elem.removeAttribute(name)
num += 1 num += 1
@ -1419,7 +1419,7 @@ def removeDuplicateGradients(doc):
# get a collection of all elements that are referenced and their referencing elements # get a collection of all elements that are referenced and their referencing elements
referencedIDs = findReferencedElements(doc.documentElement) referencedIDs = findReferencedElements(doc.documentElement)
for masterGrad in list(gradientsToRemove.keys()): for masterGrad in gradientsToRemove:
master_id = masterGrad.getAttribute('id') master_id = masterGrad.getAttribute('id')
for dupGrad in gradientsToRemove[masterGrad]: for dupGrad in gradientsToRemove[masterGrad]:
# if the duplicate gradient no longer has a parent that means it was # 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 # 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 # over style so emit them and remove them from the style map
if options.style_to_xml: if options.style_to_xml:
for propName in list(styleMap.keys()): for propName in list(styleMap):
if propName in svgAttributes: if propName in svgAttributes:
node.setAttribute(propName, styleMap[propName]) node.setAttribute(propName, styleMap[propName])
del 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)] attributes = [node.attributes.item(i).nodeName for i in range(node.attributes.length)]
for attribute in attributes: for attribute in attributes:
if attribute not in tainted: if attribute not in tainted:
if attribute in list(default_properties.keys()): if attribute in default_properties:
if node.getAttribute(attribute) == default_properties[attribute]: if node.getAttribute(attribute) == default_properties[attribute]:
node.removeAttribute(attribute) node.removeAttribute(attribute)
num += 1 num += 1
@ -1948,9 +1948,9 @@ def removeDefaultAttributeValues(node, options, tainted=set()):
tainted = taint(tainted, attribute) tainted = taint(tainted, attribute)
# Properties might also occur as styles, remove them too # Properties might also occur as styles, remove them too
styles = _getStyle(node) styles = _getStyle(node)
for attribute in list(styles.keys()): for attribute in list(styles):
if attribute not in tainted: if attribute not in tainted:
if attribute in list(default_properties.keys()): if attribute in default_properties:
if styles[attribute] == default_properties[attribute]: if styles[attribute] == default_properties[attribute]:
del styles[attribute] del styles[attribute]
num += 1 num += 1
@ -1975,7 +1975,7 @@ def convertColor(value):
""" """
s = value s = value
if s in list(colors.keys()): if s in colors:
s = colors[s] s = colors[s]
rgbpMatch = rgbp.match(s) rgbpMatch = rgbp.match(s)
@ -2031,7 +2031,7 @@ def convertColors(element):
element.setAttribute(attr, newColorValue) element.setAttribute(attr, newColorValue)
numBytes += (oldBytes - len(element.getAttribute(attr))) numBytes += (oldBytes - len(element.getAttribute(attr)))
# colors might also hide in styles # colors might also hide in styles
if attr in list(styles.keys()): if attr in styles:
oldColorValue = styles[attr] oldColorValue = styles[attr]
newColorValue = convertColor(oldColorValue) newColorValue = convertColor(oldColorValue)
oldBytes = len(oldColorValue) oldBytes = len(oldColorValue)
@ -2770,7 +2770,7 @@ def reducePrecision(element):
num += len(val) - len(newVal) num += len(val) - len(newVal)
element.setAttribute(lengthAttr, newVal) element.setAttribute(lengthAttr, newVal)
# repeat for attributes hidden in styles # repeat for attributes hidden in styles
if lengthAttr in list(styles.keys()): if lengthAttr in styles:
val = styles[lengthAttr] val = styles[lengthAttr]
valLen = SVGLength(val) valLen = SVGLength(val)
if valLen.units != Unit.INVALID: if valLen.units != Unit.INVALID: