marcus/scour
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Rewrite redundant codepatterns introduced by py2 -> py3 conversion

Browse source 
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
parent b20a0698cc
commit 0244da3fd1
1 changed files with 14 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

28
scour/scour.py
View file

@ -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: