Low-hanging performance improvements #5664

Merged
merged 7 commits into from Dec 22, 2015

Conversation

Projects
None yet
3 participants
Owner

mdboom commented Dec 11, 2015

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.

And cbook.popall has no reason to exist. Maybe someone's using it and it should stay, but no one should be using it.

On my benchmark of:

plt.subplots(8, 8)
plt.savefig("test.png")

I get a speedup from 5.04s to 3.81s.

@mdboom mdboom 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.
cd99b5b

mdboom added the needs_review label Dec 11, 2015

mdboom added some commits Dec 12, 2015

@mdboom mdboom Fix pickling
1299528
@mdboom mdboom Fix comment
0364847
@mdboom mdboom Fix pickling
49f77ad
@mdboom mdboom Fix pickle
26d3e25
@mdboom mdboom Fix pickling
5e1ee64
@mdboom mdboom PEP8
8a69511
Owner

mdboom commented Dec 22, 2015

I think this is finally ready-to-merge.

@tacaswell tacaswell commented on the diff Dec 22, 2015

lib/matplotlib/transforms.py
@@ -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
@tacaswell

tacaswell Dec 22, 2015

Owner

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

@mdboom

mdboom Dec 22, 2015

Owner

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.

@tacaswell

tacaswell Dec 22, 2015

Owner

fair enough.

@tacaswell tacaswell added a commit that referenced this pull request Dec 22, 2015

@tacaswell tacaswell Merge pull request #5664 from mdboom/performance
PRF: Low-hanging performance improvements
50ce5ff

@tacaswell tacaswell merged commit 50ce5ff into matplotlib:master Dec 22, 2015

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.08%) to 68.321%
Details

tacaswell removed the needs_review label Dec 22, 2015

@pelson pelson commented on the diff Jan 6, 2016

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)))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment