rendering slowdown with big invisible lines #1256

Closed
pierre-haessig opened this Issue Sep 14, 2012 · 12 comments

Comments

Projects
None yet
3 participants
Contributor

pierre-haessig commented Sep 14, 2012

I noticed a rendering slowdown when there is a "big" Line2D object (say 10^7
points) added to an Axes, with visibility set to false. I guess that an invisible artist should consume zero processing time instead.

Here is an tiny script meant to be run interactively (say with Python in pylab mode) step by step :
https://gist.github.com/3638471

@pelson traced the problem back to the Axes.draw method where no filtering based on visibility is currently implemented. (Although the visibility flag must be taken into account deeper in the code because the actual rendering of a 10^7 pts Line2D generates an OverflowError: Agg rendering complexity exceeded on my computer)

Reference to the discussion thread on matplotlib-devel mailing list:
http://sourceforge.net/mailarchive/forum.php?thread_name=CA%2BL60sA5_06JXDFFz9gzX-uMUj5rqh%2BJ_TMy88qpNb5NV7whnQ%40mail.gmail.com&forum_name=matplotlib-devel

Contributor

pierre-haessig commented Sep 14, 2012

Now, looking at the code path, I would more likely suspect Line2D.draw method. Indeed, it starts as:

def draw(self, renderer):
        if self._invalidy or self._invalidx:
            self.recache()
        self.ind_offset = 0  # Needed for contains() method.
        if self._subslice and self.axes:
            # Need to handle monotonically decreasing case also...
            x0, x1 = self.axes.get_xbound()
            i0, = self._x.searchsorted([x0], 'left')
            i1, = self._x.searchsorted([x1], 'right')
            subslice = slice(max(i0-1, 0), i1+1)
            self.ind_offset = subslice.start
            self._transform_path(subslice)

        transformed_path = self._get_transformed_path()

        if not self.get_visible(): return
       #[...]

This means that two operations can take place before returning : recaching and path transformation. Maybe I should try to profile this.

Member

pelson commented Sep 15, 2012

Agreed. If you put the return statement first I assume the issue would be resolved (obviously you would need to double check). If you willing and able, a pull request to do that would be great, if not, I will probably get a chance this week sometime.

Member

dmcdougall commented Sep 15, 2012

+1 on moving the return statement. Exactly what I was going to suggest.

It might be a good idea to do this for the other artists, too.

Contributor

pierre-haessig commented Sep 16, 2012

I git-cloned the source code but now I need to spend some time understanding the development toolchain.

I've seen the instructions for building mpl source code, but I didn't see a document about how to test matplotlib without installing it system wide. Is it possible to test matplotlib from within the source tree or do I need some virtualenv (which I've heard of many times but never used) ?

Member

dmcdougall commented Sep 16, 2012

I install it into my home directory and then set my PYTHONPATH shell variable. Then, if you want to ignore the installation in the home directory, I just remove $HOME from my $PYTHONPATH.

Member

dmcdougall commented Sep 16, 2012

Or, if you have other python libs installed in the home directory and you don't want to nuke them all by removing $HOME from $PYTHONPATH, you can symlink the mpl build directory into your home directory. Then if you want to uninstall it, just remove the symlink.

Member

pelson commented Sep 17, 2012

@pierre-haessig : Your question would make a great write-up. I suspect we all use different mechanisms. I for one, really dislike having to set my PYTHONPATH, therefore I find I use virtualenv, or for a quick test will sometimes do:

python setup.py install --user

Which installs things to ~/.local/ by default.

Contributor

pierre-haessig commented Oct 2, 2012

I've moved the return statement in Line2D.draw and it works. I've made a script that numerically validates the performance https://gist.github.com/3638471#file_test_line_render_speed.py (the tuning of the Python path is a bit computer-specific though)

Thanks @pelson for the python setup.py install --user tip !

I guess next step is to create a branch and submit a PR. But first I need to test that this change doesn't break something else... nose is running...

Member

dmcdougall commented Oct 2, 2012

@pierre-haessig Running nose is a good idea. I should add that when you submit a pull request, a continuous integration server will automagically run your code and the mpl test suite for you. Run time varies, but usually takes O(10 minutes).

Contributor

pierre-haessig commented Nov 24, 2012

I finally took the time to dig in this issue and I submitted a PR (#1531). Following mpl's Coding Guide, I didn't created a dedicated branch since it seems to be just a bugfix.

Also, I have to mention it took me some time to run the tests of the mpl installation in ~/.local. Tuning the PYTHONPATH variable was not working because it seems to get shortcircuited by other pathes is ``sys.path. I ended up adding one quick and dirty line in the topleveltests.py` script.

sys.path.insert(0, '/home/pierre/.local/lib/python2.7/site-packages')

I'm not fond of this solution, but it worked... ;-)

@pierre-haessig pierre-haessig added a commit to pierre-haessig/matplotlib that referenced this issue Dec 5, 2012

@pierre-haessig pierre-haessig fix rendering slowdown of big Lines (issue #1256)
About the fix:

Line2D.draw method now *returns immediately*,
if the Line is invisible (get_visible() returns False).

In particular, the `self.recache` and `self._transform_path`
calls are short-circuited.

In addition to the bugfix:

* adds a test case for lines rendering speed (issue #1256)
* adds a README in `matplotlib.tests` to redirect people
  to the testing section of the development guide.
c94c730
Member

dmcdougall commented Mar 29, 2013

This should be fixed now since #1531 was merged.

dmcdougall closed this Mar 29, 2013

Contributor

pierre-haessig commented Mar 29, 2013

That's right, I had forgot this issue ! Thanks

sfroid referenced this issue Mar 24, 2014

Merged

Issue 2899 #2923

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment