Skip to content

Axes.__init__ speedup #8626

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.

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 3 commits into from
Nov 8, 2017
Merged

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented May 16, 2017

PR Summary

As noted in #6664, Axes.__init__ is fairly slow due to the eventual call to Axis.reset_ticks. @efiring suggested lazy instantiation and @Kojoley was implementing, but I'm unsure of the status. In the meantime though, we still speed things up by avoiding calling cla too many times.

Using the example from #6664 with v2.0.x:

$ python3 -m timeit \
  -s "from matplotlib.figure import Figure; from matplotlib.axes import Axes; fig = Figure()" \
  "Axes(fig, (0, 0, 1 ,1))"
10 loops, best of 3: 29.7 msec per loop

and master is already a bit faster:

$ python -m timeit \
  -s "from matplotlib.figure import Figure; from matplotlib.axes import Axes; fig = Figure()" \
  "Axes(fig, (0, 0, 1 ,1))"
100 loops, best of 3: 17.7 msec per loop

The trouble with Axes.__init__ is that it does a bit too much clearing; Axes.__init__ calls:

  • Axes._init_axis
    • Creates self.xaxis and self.yaxis
      • Axis.__init__ -> Axis.cla (2 calls per Axes)
    • Registers each Axis with two spines via Spine.register_axis
      • Axis.cla is called on the registered Axis (2 Spine * 2 Axis = 4 calls per Axes)
  • Axes.cla - Necessary to finish initializing the Axes in a clean state, though it's generic for any caller.
    • self.xaxis.cla() / self.yaxis.cla() (2 calls per Axes)
    • self.spines[:].cla() (2 spines per Axis = 4 calls per Axes)

That makes 12 calls to Axis.cla for only 2 Axis objects. Adding some hidden argument to skip these calls as in this PR would reduce those calls by 6, though the speedup is only approximately 4 times:

$ python -m timeit \
  -s "from matplotlib.figure import Figure; from matplotlib.axes import Axes; fig = Figure()" \
  "Axes(fig, (0, 0, 1 ,1))"
100 loops, best of 3: 4.45 msec per loop

PR Checklist

  • [N/A] Has Pytest style unit tests
  • Code is PEP 8 compliant
  • [N/A] New features are documented, with examples if plot related
  • [N/A] Documentation is sphinx and numpydoc compliant
  • [N/A] Added an entry to doc/users/whats_new.rst if major new feature
  • [N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@QuLogic QuLogic added this to the 2.1 (next point release) milestone May 16, 2017
QuLogic added 3 commits May 16, 2017 01:30
Both these classes have called their own .cla() method in their
.__init__() method, so don't call it again in Axes.cla() if doing
Axes.__init__().
If the Axis was just created, calling Axis.cla is redundant because it
just happened.
@QuLogic QuLogic force-pushed the Axes-init-speedup branch from 799946c to 4801cbc Compare May 16, 2017 06:18
@QuLogic
Copy link
Member Author

QuLogic commented May 16, 2017

It turns out that subclasses (like PolarAxes) tend to override their cla method, so changing their signature is dangerous. Instead, I added an attribute that indicates that the class is still in its __init__ phase.

I'm not sure if the same is true of Spine.register_axis, but seen an error and thus haven't changed it back.

@WeatherGod
Copy link
Member

This seems awkward. Couldn't we take advantage of self.stale instead?

@QuLogic
Copy link
Member Author

QuLogic commented May 16, 2017

I don't see how. If you mean use it directly, it doesn't hold the same information as what we need. The added flag indicates (essentially) "no settings have been changed from the default", but self.stale means "current settings are different from what was drawn". It has no relationship with default settings for us to use.

If you mean, write something similar to self.stale, that's a lot of work for very little gain and seems rather non-trivial and error-prone to me.

@tacaswell tacaswell modified the milestones: 2.2 (next next feature release), 2.1 (next point release) Aug 29, 2017
@efiring
Copy link
Member

efiring commented Nov 8, 2017

I'm starting to think that the approach to reducing the redundant cla calls here is better than the way I did it in #8752. If so, we should merge this (or a modification of it) and I can change #8752 accordingly. (It needs some more work, but I still want to get that one in, too.)

Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

I think this looks like a clean and minimally-invasive solution to a long-standing problem.

@tacaswell tacaswell merged commit 1636bed into matplotlib:master Nov 8, 2017
@QuLogic QuLogic deleted the Axes-init-speedup branch November 8, 2017 04:11
@dstansby
Copy link
Member

dstansby commented Nov 8, 2017

This appears to have broken lots of stuff on travis! https://travis-ci.org/matplotlib/matplotlib/builds/298909059

I'm going to revert, @QuLogic could you re-open the PR so we can run the changes against the latest revision of master?

@Kojoley
Copy link
Member

Kojoley commented Nov 8, 2017

This reminds me all kind of strange fails I had faced a year ago at fixing the particular problem.

@QuLogic QuLogic restored the Axes-init-speedup branch November 21, 2017 09:52
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 13, 2018
@QuLogic QuLogic deleted the Axes-init-speedup branch April 16, 2021 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants