Skip to content

Low-hanging performance improvements #5664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 22, 2015
Merged

Low-hanging performance improvements #5664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cd99b5b
Low-hanging performance improvements
mdboom Dec 11, 2015
1299528
Fix pickling
mdboom Dec 12, 2015
0364847
Fix comment
mdboom Dec 14, 2015
49f77ad
Fix pickling
mdboom Dec 15, 2015
26d3e25
Fix pickle
mdboom Dec 15, 2015
5e1ee64
Fix pickling
mdboom Dec 15, 2015
8a69511
PEP8
mdboom Dec 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/matplotlib/axis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)])
Expand Down
6 changes: 0 additions & 6 deletions lib/matplotlib/cbook.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion lib/matplotlib/markers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@
(TICKLEFT, TICKRIGHT, TICKUP, TICKDOWN,
CARETLEFT, CARETRIGHT, CARETUP, CARETDOWN) = list(xrange(8))

_empty_path = Path(np.empty((0, 2)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, there is a fast instantiation route for Paths: https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/path.py#L173.



class MarkerStyle(object):

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/tests/test_pickle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(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)
Expand Down
41 changes: 25 additions & 16 deletions lib/matplotlib/transforms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
update_path_extents)
from numpy.linalg import inv

from weakref import WeakValueDictionary
import weakref
import warnings
try:
set
Expand Down Expand Up @@ -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.
Expand All @@ -109,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a check that v() returns an active ref?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I currently check on deserializing (__setstate__) where you have to deal with it. No real need to do it in both places unless we really care about the size of the serialization.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough.

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) if v is not None)

def __copy__(self, *args):
raise NotImplementedError(
Expand Down Expand Up @@ -156,8 +156,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):
"""
Expand All @@ -166,8 +169,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
Expand Down Expand Up @@ -1560,8 +1566,9 @@ 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):
Expand All @@ -1570,8 +1577,10 @@ 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
self._parents = WeakValueDictionary(state['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(state['parents']) if v is not None)

def __repr__(self):
return "TransformWrapper(%r)" % self._child
Expand Down