From 8c9c6253b9b9b4328b191740130275966c71c876 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Sat, 17 Feb 2018 13:44:44 +0000
Subject: [PATCH] Move loop inside a function to reduce the number of calls

In a profile of scour on a given input file,
removeDefaultAttributeValue is one of the top 3 most frequent function
being called with 332288 calls (with minidom.getAttribute and
minidom.hasAttribute being the only ones with similar number of
calls).

The high number of calls occurs because removeDefaultAttributeValue
only removes a single attribute at a time.  Given it is always called
in a trivial for loop, we can just move the loop inside
removeDefaultAttributeValue to reduce the number of calls.  At the
same time, rename the function to better reflect that it works on
multiple attributes.

Signed-off-by: Niels Thykier <niels@thykier.net>
---
 scour/scour.py | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/scour/scour.py b/scour/scour.py
index 16af5b5..d5c9e8a 100644
--- a/scour/scour.py
+++ b/scour/scour.py
@@ -1896,34 +1896,38 @@ def taint(taintedSet, taintedAttribute):
     return taintedSet
 
 
-def removeDefaultAttributeValue(node, attribute):
+def removeDefaultAttributeValuesFromNode(node, attributes):
     """
-    Removes the DefaultAttribute 'attribute' from 'node' if specified conditions are fulfilled
+    Removes each of the 'DefaultAttribute's from 'node' if specified conditions are fulfilled
     """
-    if not node.hasAttribute(attribute.name):
-        return 0
+    count = 0
+    for attribute in attributes:
+        if not node.hasAttribute(attribute.name):
+            continue
 
-    if (attribute.elements is not None) and (node.nodeName not in attribute.elements):
-        return 0
+        if (attribute.elements is not None) and (node.nodeName not in attribute.elements):
+            continue
 
-    # differentiate between text and numeric values
-    if isinstance(attribute.value, str):
-        if node.getAttribute(attribute.name) == attribute.value:
-            if (attribute.conditions is None) or attribute.conditions(node):
-                node.removeAttribute(attribute.name)
-                return 1
-    else:
-        nodeValue = SVGLength(node.getAttribute(attribute.name))
-        if ((attribute.value is None)
-                or ((nodeValue.value == attribute.value) and not (nodeValue.units == Unit.INVALID))):
-            if ((attribute.units is None)
-                    or (nodeValue.units == attribute.units)
-                    or (isinstance(attribute.units, list) and nodeValue.units in attribute.units)):
+        # differentiate between text and numeric values
+        if isinstance(attribute.value, str):
+            if node.getAttribute(attribute.name) == attribute.value:
                 if (attribute.conditions is None) or attribute.conditions(node):
                     node.removeAttribute(attribute.name)
-                    return 1
+                    count += 1
+                    continue
+        else:
+            nodeValue = SVGLength(node.getAttribute(attribute.name))
+            if ((attribute.value is None)
+                    or ((nodeValue.value == attribute.value) and not (nodeValue.units == Unit.INVALID))):
+                if ((attribute.units is None)
+                        or (nodeValue.units == attribute.units)
+                        or (isinstance(attribute.units, list) and nodeValue.units in attribute.units)):
+                    if (attribute.conditions is None) or attribute.conditions(node):
+                        node.removeAttribute(attribute.name)
+                        count += 1
+                        continue
 
-    return 0
+    return count
 
 
 def removeDefaultAttributeValues(node, options, tainted=set()):
@@ -1935,8 +1939,7 @@ def removeDefaultAttributeValues(node, options, tainted=set()):
         return 0
 
     # Conditionally remove all default attributes defined in 'default_attributes' (a list of 'DefaultAttribute's)
-    for attribute in default_attributes:
-        num += removeDefaultAttributeValue(node, attribute)
+    num += removeDefaultAttributeValuesFromNode(node, default_attributes)
 
     # Summarily get rid of default properties
     attributes = [node.attributes.item(i).nodeName for i in range(node.attributes.length)]