From cd99b5b32e3616fb01ec5cfa001bb898820db622 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Fri, 11 Dec 2015 17:16:45 -0500 Subject: [PATCH 1/7] Low-hanging performance improvements Instantiating Paths is expensive, but they aren't mutable, so just reuse when possible. WeakValueDictionaries are expensive to construct. We can fix that by handling the weakrefs ourself. --- lib/matplotlib/axis.py | 4 ++-- lib/matplotlib/cbook.py | 6 ------ lib/matplotlib/markers.py | 4 +++- lib/matplotlib/transforms.py | 19 +++++++++++-------- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/matplotlib/axis.py b/lib/matplotlib/axis.py index 3c578d431adc..dfae60d872d8 100644 --- a/lib/matplotlib/axis.py +++ b/lib/matplotlib/axis.py @@ -750,8 +750,8 @@ def reset_ticks(self): # build a few default ticks; grow as necessary later; only # define 1 so properties set on ticks will be copied as they # grow - cbook.popall(self.majorTicks) - cbook.popall(self.minorTicks) + del self.majorTicks[:] + del self.minorTicks[:] self.majorTicks.extend([self._get_tick(major=True)]) self.minorTicks.extend([self._get_tick(major=False)]) diff --git a/lib/matplotlib/cbook.py b/lib/matplotlib/cbook.py index b907ec01efbb..077337f6d34c 100644 --- a/lib/matplotlib/cbook.py +++ b/lib/matplotlib/cbook.py @@ -1392,12 +1392,6 @@ def remove(self, o): self.push(thiso) -def popall(seq): - 'empty a list' - for i in xrange(len(seq)): - seq.pop() - - def finddir(o, match, case=False): """ return all attributes of *o* which match string in match. if case diff --git a/lib/matplotlib/markers.py b/lib/matplotlib/markers.py index 8eef2ae20ebd..3a5494c0ed58 100644 --- a/lib/matplotlib/markers.py +++ b/lib/matplotlib/markers.py @@ -93,6 +93,8 @@ (TICKLEFT, TICKRIGHT, TICKUP, TICKDOWN, CARETLEFT, CARETRIGHT, CARETUP, CARETDOWN) = list(xrange(8)) +_empty_path = Path(np.empty((0, 2))) + class MarkerStyle(object): @@ -182,7 +184,7 @@ def __setstate__(self, statedict): self._recache() def _recache(self): - self._path = Path(np.empty((0, 2))) + self._path = _empty_path self._transform = IdentityTransform() self._alt_path = None self._alt_transform = None diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index a7accb320621..8603f4a773d7 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -40,7 +40,7 @@ update_path_extents) from numpy.linalg import inv -from weakref import WeakValueDictionary +import weakref import warnings try: set @@ -92,10 +92,7 @@ def __init__(self, shorthand_name=None): other than to improve the readability of ``str(transform)`` when DEBUG=True. """ - # Parents are stored in a WeakValueDictionary, so that if the - # parents are deleted, references from the children won't keep - # them alive. - self._parents = WeakValueDictionary() + self._parents = {} # TransformNodes start out as invalid until their values are # computed for the first time. @@ -156,8 +153,11 @@ def _invalidate_internal(self, value, invalidating_node): self._invalid = value for parent in list(six.itervalues(self._parents)): - parent._invalidate_internal(value=value, - invalidating_node=self) + # Dereference the weak reference + parent = parent() + if parent is not None: + parent._invalidate_internal( + value=value, invalidating_node=self) def set_children(self, *children): """ @@ -166,8 +166,11 @@ def set_children(self, *children): Should be called from the constructor of any transforms that depend on other transforms. """ + # Parents are stored as weak references, so that if the + # parents are destroyed, references from the children won't + # keep them alive. for child in children: - child._parents[id(self)] = self + child._parents[id(self)] = weakref.ref(self) if DEBUG: _set_children = set_children From 1299528a12ec9dcfa234664e03d5b203b2cec809 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Fri, 11 Dec 2015 20:11:49 -0500 Subject: [PATCH 2/7] Fix pickling --- lib/matplotlib/transforms.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index 8603f4a773d7..1c812ca917ee 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -106,14 +106,17 @@ def __str__(self): def __getstate__(self): d = self.__dict__.copy() - # turn the weakkey dictionary into a normal dictionary - d['_parents'] = dict(six.iteritems(self._parents)) + # turn the dictionary with weak values into a normal dictionary + d['_parents'] = dict((k, v()) for (k, v) in + six.iteritems(self._parents)) return d def __setstate__(self, data_dict): self.__dict__ = data_dict - # turn the normal dictionary back into a WeakValueDictionary - self._parents = WeakValueDictionary(self._parents) + # turn the normal dictionary back into a dictionary with weak + # values + self._parents = dict((k, weakref.ref(v)) for (k, v) in + six.iteritems(self._parents)) def __copy__(self, *args): raise NotImplementedError( @@ -1563,8 +1566,8 @@ def __getstate__(self): 'child': self._child, 'input_dims': self.input_dims, 'output_dims': self.output_dims, - # turn the weakkey dictionary into a normal dictionary - 'parents': dict(six.iteritems(self._parents)) + # turn the weak-values dictionary into a normal dictionary + 'parents': dict((k, v()) for (k, v) in six.iteritems(self._parents)) } def __setstate__(self, state): @@ -1574,7 +1577,7 @@ def __setstate__(self, state): self.input_dims = state['input_dims'] self.output_dims = state['output_dims'] # turn the normal dictionary back into a WeakValueDictionary - self._parents = WeakValueDictionary(state['parents']) + self._parents = dict((k, weakref.ref(v)) for (k, v) in state['parents']) def __repr__(self): return "TransformWrapper(%r)" % self._child From 0364847f70c27126c55b5ef47923787ae5345c43 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Mon, 14 Dec 2015 10:38:32 -0500 Subject: [PATCH 3/7] Fix comment --- lib/matplotlib/transforms.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index 1c812ca917ee..abbc43078650 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -1576,7 +1576,8 @@ def __setstate__(self, state): # The child may not be unpickled yet, so restore its information. self.input_dims = state['input_dims'] self.output_dims = state['output_dims'] - # turn the normal dictionary back into a WeakValueDictionary + # turn the normal dictionary back into a dictionary with weak + # values self._parents = dict((k, weakref.ref(v)) for (k, v) in state['parents']) def __repr__(self): From 49f77ad52db2fb2b3b86ca6c307c8dfabf479697 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 15 Dec 2015 08:36:08 -0500 Subject: [PATCH 4/7] Fix pickling --- lib/matplotlib/transforms.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index abbc43078650..7ac3d47e32c7 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -1578,7 +1578,8 @@ def __setstate__(self, state): self.output_dims = state['output_dims'] # turn the normal dictionary back into a dictionary with weak # values - self._parents = dict((k, weakref.ref(v)) for (k, v) in state['parents']) + self._parents = dict((k, weakref.ref(v)) for (k, v) in + six.iteritems(state['parents'])) def __repr__(self): return "TransformWrapper(%r)" % self._child From 26d3e2550ed3c41af67307ff1670cffa01442e66 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 15 Dec 2015 08:46:21 -0500 Subject: [PATCH 5/7] Fix pickle --- lib/matplotlib/tests/test_pickle.py | 2 +- lib/matplotlib/transforms.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/tests/test_pickle.py b/lib/matplotlib/tests/test_pickle.py index 3419b82c2822..e740f97ba331 100644 --- a/lib/matplotlib/tests/test_pickle.py +++ b/lib/matplotlib/tests/test_pickle.py @@ -278,7 +278,7 @@ def test_transform(): # Check parent -> child links of TransformWrapper. assert_equal(obj.wrapper._child, obj.composite) # Check child -> parent links of TransformWrapper. - assert_equal(list(obj.wrapper._parents.values()), [obj.composite2]) + assert_equal([v() for v in obj.wrapper._parents.values()], [obj.composite2]) # Check input and output dimensions are set as expected. assert_equal(obj.wrapper.input_dims, obj.composite.input_dims) assert_equal(obj.wrapper.output_dims, obj.composite.output_dims) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index 7ac3d47e32c7..f6c9c77cd172 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -1567,7 +1567,8 @@ def __getstate__(self): 'input_dims': self.input_dims, 'output_dims': self.output_dims, # turn the weak-values dictionary into a normal dictionary - 'parents': dict((k, v()) for (k, v) in six.iteritems(self._parents)) + 'parents': dict((k, v()) for (k, v) in + six.iteritems(self._parents)) } def __setstate__(self, state): From 5e1ee64b3d4652bf7e122c0335130807483d8965 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 15 Dec 2015 11:32:52 -0500 Subject: [PATCH 6/7] Fix pickling --- lib/matplotlib/transforms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index f6c9c77cd172..7dd04c1e26f4 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -116,7 +116,7 @@ def __setstate__(self, data_dict): # turn the normal dictionary back into a dictionary with weak # values self._parents = dict((k, weakref.ref(v)) for (k, v) in - six.iteritems(self._parents)) + six.iteritems(self._parents) if v is not None) def __copy__(self, *args): raise NotImplementedError( @@ -1580,7 +1580,7 @@ def __setstate__(self, state): # turn the normal dictionary back into a dictionary with weak # values self._parents = dict((k, weakref.ref(v)) for (k, v) in - six.iteritems(state['parents'])) + six.iteritems(state['parents']) if v is not None) def __repr__(self): return "TransformWrapper(%r)" % self._child From 8a69511753458be7aa0fd2795a2716f1e03d011a Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 22 Dec 2015 13:30:55 -0500 Subject: [PATCH 7/7] PEP8 --- lib/matplotlib/tests/test_pickle.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/tests/test_pickle.py b/lib/matplotlib/tests/test_pickle.py index e740f97ba331..da2e7f699e90 100644 --- a/lib/matplotlib/tests/test_pickle.py +++ b/lib/matplotlib/tests/test_pickle.py @@ -278,7 +278,8 @@ def test_transform(): # Check parent -> child links of TransformWrapper. assert_equal(obj.wrapper._child, obj.composite) # Check child -> parent links of TransformWrapper. - assert_equal([v() for v in obj.wrapper._parents.values()], [obj.composite2]) + assert_equal( + [v() for v in obj.wrapper._parents.values()], [obj.composite2]) # Check input and output dimensions are set as expected. assert_equal(obj.wrapper.input_dims, obj.composite.input_dims) assert_equal(obj.wrapper.output_dims, obj.composite.output_dims)