Make Collection.set_paths even faster #16793

apaszke · 2020-03-16T20:38:29Z

PR Summary

As in the title, this PR optimizes Collection.set_paths even further. With all PRs I've submitted before this one applied, the rendering for my example (400x400 surface plot on a 2012 MBP) takes ~2s on average. With this patch, the time drops to ~0.7s. The breakdown of this 0.7s is:

~0.4s in Poly3DCollection.do_3d_projection
- Including ~0.2s in Collection.set_paths
~0.3s in Collection.draw (mostly spend in the C code for the Agg backend in this case)

PR Checklist

Has Pytest style unit tests
Code is Flake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

apaszke · 2020-03-16T20:39:00Z

lib/matplotlib/path.py

@@ -74,8 +74,12 @@ class Path:
        made up front in the constructor that will not change when the
        data changes.
    """
+    __slots__ = ('_vertices', '_codes', '_readonly',
+                 '_should_simplify', '_simplify_threshold',
+                 '_interpolation_steps')


Adding slots significantly speeds up the time it takes to create an instance!

apaszke · 2020-03-16T20:39:53Z

lib/matplotlib/path.py

        """
        pth = cls.__new__(cls)
-        pth._vertices = _to_unmasked_float_array(verts)
+        if unmask_verts:
+            pth._vertices = _to_unmasked_float_array(verts)


Apparently this function is not as fast due to an extra call overhead, and because _to_unmasked_float_array calls np.asarray, which is pretty expensive even when you pass in an array!

See numpy/numpy#13580 :) how much do we get if we just add in _to_unmasked_float_array a check to return things as if the arg is already an array?
Also, I think the warrants a comment explaining that this is for perf. reasons.

I'm pretty sure that the extra call adds a fair amount of overhead itself, and a robust check would not be cheap either. I can try and circle back with the results if you want.

lib/matplotlib/path.py

apaszke · 2020-03-16T20:42:02Z

lib/matplotlib/collections.py

+            example_path = mpath.Path(verts_pad[0], codes)
+            # Looking up the function once before the iteration gives a nice speedup
+            _make_path = mpath.Path._fast_from_codes_and_verts
+            self._paths = [_make_path(xy, codes,


Turns out that the Python function call overheads become quite significant here, and so inlining the body of _fast_from_codes_and_verts into this function actually results in an end-to-end 10% improvement for my example plot (this function along becomes almost 2x faster)! On the other hand, it's a method that touches all the Path internals, so I would feel a bit weird inlining it here...

QuLogic · 2020-03-16T22:24:55Z

This appears to have broken some things.

apaszke · 2020-03-20T12:09:56Z

@QuLogic Whoops, I ran way too small of a test subset before I've pushed this. Should be good now!

lib/matplotlib/tests/test_transforms.py

lib/matplotlib/collections.py

lib/matplotlib/cbook/__init__.py

QuLogic

What are you using as a benchmark?

lib/matplotlib/cbook/__init__.py

lib/matplotlib/path.py

QuLogic · 2020-03-24T23:44:26Z

lib/matplotlib/path.py

@@ -160,7 +164,8 @@ def __init__(self, vertices, codes=None, _interpolation_steps=1,
            self._readonly = False

    @classmethod
-    def _fast_from_codes_and_verts(cls, verts, codes, internals_from=None):
+    def _fast_from_codes_and_verts(cls, verts, codes, internals_from=None,
+                                   unmask_verts=True):


This is a private function; do we even need the flag? Maybe a check for masked arrays is needed, but I'm pretty sure all internal callers already provide arrays.

lib/matplotlib/tests/test_transforms.py

apaszke

@QuLogic The same snippet as in all the other PRs:

import io                                                             
import time                                                           
import numpy as np                                                    
from mpl_toolkits.mplot3d import Axes3D                               
import matplotlib.pyplot as plt                                             
from contextlib import contextmanager                                 
from collections import defaultdict                                   
                                                                      
results = defaultdict(lambda: np.zeros((2,)))                         
@contextmanager                                                       
def measure(name):                                                    
    s = time.perf_counter()                                           
    yield                                                             
    d = time.perf_counter() - s                                       
    results[name] += [1, d]                                           
                                                                      
def summarize():                                                      
    for name, (count, total) in sorted(results.items()):              
        print(name, total / count)                                    
                                                                      
x = y = np.linspace(-1, 1, 400)                                       
x, y = np.meshgrid(x, y)                                              
z = x ** 2 + y ** 2                                                   
                                                                      
                                                                      
for i in range(4):                                                    
    fig = plt.figure()                                                
    ax = fig.gca(projection='3d')                                     
    with measure('plot'):                                             
        ax.plot_surface(x, y, z, rstride=1, cstride=1)
    with measure('draw'):                                             
        buf = io.BytesIO()                                            
        fig.savefig(buf, format='raw')                                
    plt.close()                                                       
                                                                      
summarize()

lib/matplotlib/tests/test_transforms.py

apaszke · 2020-03-25T09:18:45Z

@QuLogic This should have addressed all your comments.

QuLogic · 2020-04-13T23:06:13Z

lib/matplotlib/collections.py

+            codes = example_path.codes
+            self._paths = [_make_path(xy, codes,
+                                      internals_from=example_path,
+                                      unmask_verts=False)


unmask_verts is no longer there.

QuLogic · 2020-04-13T23:08:37Z

lib/matplotlib/path.py

@@ -175,7 +179,7 @@ def _fast_from_codes_and_verts(cls, verts, codes, internals_from=None):
            never copied, and always set to ``False`` by this constructor.
        """
        pth = cls.__new__(cls)
-        pth._vertices = _to_unmasked_float_array(verts)
+        pth._vertices = verts


Can you add a note to the docstring that verts should be unmasked, and not just any NumPy array?

QuLogic · 2020-04-28T04:06:40Z

You have ~5 days to rebase before 3.3 release candidate comes out.

QuLogic · 2020-05-27T06:19:10Z

Well, apparently not 5 days, but you still have a chance here.

tacaswell · 2020-06-09T18:29:40Z

Moved to 3.4

jklymak · 2020-09-10T15:42:47Z

ping @apaszke still interested in this optimization?

tacaswell · 2023-05-24T20:04:52Z

Coming back to this, I am 👍🏻 on the __slots__, but 👎🏻 on adding a decorator to disable gc. I'm mixed on inlining `_make_path.

scottshambaugh · 2025-01-07T22:32:17Z

I've picked this up in #29398, building off this PR (but without the garbage collection change). Thank you @apaszke for the initial work here!

apaszke commented Mar 16, 2020

View reviewed changes

QuLogic added status: needs rebase status: needs revision Performance labels Mar 16, 2020

QuLogic added this to the v3.3.0 milestone Mar 16, 2020

apaszke force-pushed the optimize_paths branch from 7a1bae7 to d8a0175 Compare March 19, 2020 22:23

apaszke commented Mar 20, 2020

View reviewed changes

lib/matplotlib/tests/test_transforms.py Outdated Show resolved Hide resolved

apaszke mentioned this pull request Mar 20, 2020

Speeding up Axes3D.plot_surface 4-8x #16659

Closed

timhoffm removed the status: needs rebase label Mar 23, 2020

timhoffm reviewed Mar 23, 2020

View reviewed changes

lib/matplotlib/collections.py Outdated Show resolved Hide resolved

lib/matplotlib/collections.py Outdated Show resolved Hide resolved

lib/matplotlib/cbook/__init__.py Show resolved Hide resolved

apaszke force-pushed the optimize_paths branch from fb081b6 to d355895 Compare March 23, 2020 11:47

Make Collection.set_paths even faster

230325d

apaszke force-pushed the optimize_paths branch from d355895 to 230325d Compare March 23, 2020 11:49

QuLogic reviewed Mar 24, 2020

View reviewed changes

apaszke commented Mar 25, 2020

View reviewed changes

lib/matplotlib/tests/test_transforms.py Outdated Show resolved Hide resolved

Review comments

3b59fb0

QuLogic reviewed Apr 13, 2020

View reviewed changes

QuLogic added the status: needs rebase label Apr 13, 2020

tacaswell modified the milestones: v3.3.0, v3.4.0 Jun 9, 2020

jklymak marked this pull request as draft September 10, 2020 15:42

QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021

QuLogic modified the milestones: v3.5.0, v3.6.0 Aug 18, 2021

timhoffm added the status: orphaned PR label Apr 30, 2022

timhoffm modified the milestones: v3.6.0, unassigned Apr 30, 2022

story645 modified the milestones: unassigned, needs sorting Oct 6, 2022

scottshambaugh mentioned this pull request Jan 3, 2025

Speed up Collection.set_paths #29398

Merged

2 tasks

scottshambaugh added the status: superseded label Jan 7, 2025

scottshambaugh marked this pull request as ready for review January 7, 2025 22:33

scottshambaugh closed this Jan 7, 2025

Uh oh!

Make Collection.set_paths even faster #16793

Make Collection.set_paths even faster #16793

Uh oh!

Conversation

apaszke commented Mar 16, 2020

PR Summary

PR Checklist

Uh oh!

apaszke Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

apaszke Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

apaszke Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

apaszke Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Mar 16, 2020

Uh oh!

apaszke commented Mar 20, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

QuLogic Mar 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

apaszke left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

apaszke commented Mar 25, 2020

Uh oh!

QuLogic Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

QuLogic Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Apr 28, 2020

Uh oh!

QuLogic commented May 27, 2020

Uh oh!

tacaswell commented Jun 9, 2020

Uh oh!

jklymak commented Sep 10, 2020

Uh oh!

tacaswell commented May 24, 2023

Uh oh!

scottshambaugh commented Jan 7, 2025

Uh oh!

Uh oh!

apaszke Mar 16, 2020 •

edited

Loading

apaszke Mar 17, 2020 •

edited

Loading

apaszke Mar 16, 2020 •

edited

Loading

apaszke left a comment •

edited

Loading