Major speed regression introduced in "plt.bar" definition clipping between 3.0.3 and 3.3.0. · Issue #17974 · matplotlib/matplotlib · GitHub

Bug report

Bug summary

As I updated Matplolib from version 3.0.3 to version 3.3.0 I noticed that Colour unit test time skyrocketed: our plotting sub-package test suite went from 120 seconds to around 3000 seconds!

I quickly noticed that it was related to definitions using the spectrum plot:

This definition is using plt.bar and as I suspected that polygon clipping might be the culprit, I ran some tests on one of the offending definitions while keeping and removing the polygon clipping:

With Polygon Clipping

>>> import timeit
>>> import colour
>>> timeit.timeit(lambda : colour.plotting.plot_single_illuminant_sd(filename='test.png'), number=1)
59.81594165299998

Without Polygon Clipping

>>> import timeit
>>> import colour
>>> timeit.timeit(lambda : colour.plotting.plot_single_illuminant_sd(filename='test.png'), number=1)
1.3159556400000056

Code for reproduction

I haven't created a reproducible case but my assumption is that the clipping code is orders of magnitude slower than before, the definition I used is available here: https://github.com/colour-science/colour/blob/d5f68005f62fc86ba59745bd4c8bb8a01bb5dcb4/colour/plotting/colorimetry.py#L183 and the meat is roughly as follows:

    polygon = Polygon(
        np.vstack([
            (x_min, 0),
            tstack([wavelengths, values]),
            (x_max, 0),
        ]),
        facecolor='none',
        edgecolor='none')
    axes.add_patch(polygon)

    padding = 0.1
    axes.bar(
        x=wavelengths - padding,
        height=max(values),
        width=1 + padding,
        color=colours,
        align='edge',
        clip_path=polygon)

There are roughly 450 wavelengths and each one of them generates a coloured bar.

Matplotlib version

Operating system: macOS 10.15.5
Matplotlib version: 3.3.0
Matplotlib backend (print(matplotlib.get_backend())): MacOSX
Python version: 3.8
Jupyter version (if applicable):
Other libraries:

Matplotlib was installed with Pip.

It'll be a lot more work for us to reproduce accurately if you don't have a minimal example.

I will workout a self-contained example tonight, but the problem is identified, it is usage of clip_path in plt.bar.

It was actually a bit harder to reproduce than expected because it is actually a combination of two things:

clip_path=polygon in the call to plt.bar
figure.tight_layout()

If both that argument and call are present, computation time explodes.

Here is a reproducible snippet that should be self-contained:

import matplotlib.pyplot as plt
import numpy as np
import timeit
from matplotlib.patches import Polygon


def tstack(a):
    return np.concatenate([x[..., np.newaxis] for x in a], axis=-1)


def plot_slow_bar():
    figure = plt.figure(figsize=(10.24, 7.68))
    axes = figure.gca()
    x_min, x_max = 360, 780
    wl = np.arange(x_min, x_max, 1)
    wl_c = len(wl)
    values = np.sin(wl / 50) * 125 + 125
    colours = np.random.random([wl_c, 3])

    polygon = Polygon(
        np.vstack([
            (x_min, 0),
            tstack([wl, values]),
            (x_max, 0),
        ]),
        facecolor='none',
        edgecolor='none')
    axes.add_patch(polygon)

    padding = 0.1
    axes.bar(
        x=wl - padding,
        height=max(values),
        width=1 + padding,
        color=colours,
        align='edge',
        # Comment this line and everything is fast.
        clip_path=polygon)

    axes.plot(wl, values)
    axes.set_xlim(x_min, x_max)
    axes.set_ylim(0, 250)

    # Comment this line and everything is fast.
    figure.tight_layout()

    plt.savefig('test.png')

print(timeit.timeit(plot_slow_bar, number=1))

Without figure.tight_layout():

(colour-49B8_mty-py3.8) Kali:colour kelsolaar$ python bar.py
2020-07-21 17:19:58.101 Python[37343:7154668] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to (null)
1.125038752

With figure.tight_layout():

(colour-49B8_mty-py3.8) Kali:colour kelsolaar$ python bar.py
2020-07-21 17:33:51.738 Python[37981:7167310] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to (null)
68.036350134

With figure.tight_layout() but no clip_path=polygon:

(colour-49B8_mty-py3.8) Kali:colour kelsolaar$ python bar.py
2020-07-21 17:35:36.849 Python[38057:7168895] ApplePersistenceIgnoreState: Existing state will not be touched. New state will be written to (null)
1.096855527

Call graph for my real function which confirms that most of the time is spent in figure.tight_layout and its dependencies:

Thanks for the example and profile! Could you try and narrow down a bit more which version this changed by testing if the regression is present in the 3.1 and 3.2 releases?

I suspect what changed was #10682 which added all artists to get_tight_bbox for an axes. Previoisly the 450 bars would have been ignored in that calculation. Not that 450 bars should take that long to compute.

One workaround, in addition to the two PRs @anntzer has put in is to take the artist out of the layout: b = ax.bar(); b.set_in_layout(False)`.

Hmmm, so I get for
v3.1.0: 1.0475010080000002
v3.2.0: 0.8915242379999999,
v3.3.0: 54.807

So this was not #10682.

bisects to 95a530c #16832 @brunobeltran

On master this now returns 0.74 seconds, so I guess short circuiting the changes in #16832. Not sure though if that points to further optimizations, or if this should just be closed.

EDIT: misread @jklymak's comment, didn't realize we're now short-circuiting correctly again. Feel free to weigh in on whether the below is still worth it....not sure why we weren't short-circuiting in the first place. tl;dr: this code is definitely much slower, but it's supposed to only be slower when you need it to be (because previously you would have gotten incorrect results). So I'm happy to add some simple optimizations to work towards that goal if they're actually useful.

There are definitely some simple optimization options to do here.

The reason that most of the code didn't get a huge speed hit is that we only ever hit this code path in the first place first place if we're not clipping to the axis box anway. I haven't had a free minute to deep dive this this test case, but I assume that adding a manual clip path is what's triggering the savefig call to use the new code here.

One (no so elegant) solution would be keep augmenting this clipping logic, since obviously in this case the curves being checked are still inside of the axis bbox, so there should be some way to short-circuit the whole looping-over-each-curve in Python for its extents.

On the other hand, the slowdown basically results from calling Python code to iterate over the vertices of each Path instead of C++ (because the C++ code only works for straight lines, so not for markers, etc). So, for cases like this where we are working with straight lines, falling back to the C++ would in principle recover all of the speed lost. A simple only_lines flag or test could be added to Path to allow us to fall back to the C++ code when it will return the same results:

if not self.codes or not np.isin(self.codes, [CURVE3, CURVE4]):
    return _path.get_extents(...)
# otherwise use new code

In principle, this would add one np.isin worth of overhead per Path compared to the 3.2 code. I'll see how much adding either or both of these fixes the problem.

Of course I could always just port the new code back into C++, but I think that only makes sense if there's not an easy solution like the above.

The above two PRs are now backported, so this should be fixed for 3.3.1.

Thanks @QuLogic ! 👍

Checked just to be sure:

In [1]: import colour                                                                                                                                                

In [2]: import timeit                                                                                                                                                

In [3]: timeit.timeit(lambda : colour.plotting.plot_single_illuminant_sd(filename='test.png'), number=1)                                                             
Out[3]: 0.9347890290009673

In [4]: import matplotlib                                                                                                                                            

In [5]: matplotlib.__version__                                                                                                                                       
Out[5]: '3.3.0+60.g99081362af'

In [6]:

Major speed regression introduced in "plt.bar" definition clipping between 3.0.3 and 3.3.0. · Issue #17974 · matplotlib/matplotlib · GitHub

Repository navigation

Major speed regression introduced in "plt.bar" definition clipping between 3.0.3 and 3.3.0. #17974

Bug report

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Major speed regression introduced in "plt.bar" definition clipping between 3.0.3 and 3.3.0. · Issue #17974 · matplotlib/matplotlib · GitHub

Repository navigation

Major speed regression introduced in "plt.bar" definition clipping between 3.0.3 and 3.3.0. #17974

Description

Bug report

Activity

jklymak commented on Jul 20, 2020

thomasmansencal commented on Jul 20, 2020

thomasmansencal commented on Jul 21, 2020

dstansby commented on Jul 21, 2020

jklymak commented on Jul 21, 2020

jklymak commented on Jul 21, 2020

brunobeltran commented on Jul 22, 2020

QuLogic commented on Aug 5, 2020

thomasmansencal commented on Aug 6, 2020

tacaswell commented on Aug 6, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions