Skip to content

Make Collection.set_paths even faster #16793

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Make Collection.set_paths even faster #16793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
230325d
Make Collection.set_paths even faster
apaszke Mar 16, 2020
3b59fb0
Review comments
apaszke Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/matplotlib/cbook/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import collections.abc
import contextlib
import functools
import gc
import gzip
import itertools
import operator
Expand Down Expand Up @@ -2343,3 +2344,17 @@ def _backend_module_name(name):
"""
return (name[9:] if name.startswith("module://")
else "matplotlib.backends.backend_{}".format(name.lower()))


def _without_gc(f):
"""A decorator to disable garbage collection in the decorated function."""
@functools.wraps(f)
def wrapper(*args, **kwargs):
was_enabled = gc.isenabled()
gc.disable()
try:
return f(*args, **kwargs)
finally:
if was_enabled:
gc.enable()
return wrapper
22 changes: 15 additions & 7 deletions lib/matplotlib/collections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,10 @@ def __init__(self, verts, sizes=None, closed=True, **kwargs):
self.set_verts(verts, closed)
self.stale = True

# This function creates a lot of Path object, which is pretty much
# guaranteed to trigger the GC multiple times. None of them will be garbage
# just yet, so all those runs are completely unnecessary.
@cbook._without_gc
def set_verts(self, verts, closed=True):
"""
Set the vertices of the polygons.
Expand All @@ -1094,15 +1098,19 @@ def set_verts(self, verts, closed=True):
return

# Fast path for arrays
if isinstance(verts, np.ndarray):
verts_pad = np.concatenate((verts, verts[:, :1]), axis=1)
if isinstance(verts, np.ndarray) and len(verts):
verts_pad = (np.concatenate((verts, verts[:, :1]), axis=1)
.astype(mpath.Path.vert_type))
# Creating the codes once is much faster than having Path do it
# separately each time by passing closed=True.
codes = np.empty(verts_pad.shape[1], dtype=mpath.Path.code_type)
codes[:] = mpath.Path.LINETO
codes[0] = mpath.Path.MOVETO
codes[-1] = mpath.Path.CLOSEPOLY
self._paths = [mpath.Path(xy, codes) for xy in verts_pad]
example_path = mpath.Path(verts_pad[0], closed=True)
# Looking up the values once speeds up the iteration a bit
_make_path = mpath.Path._fast_from_codes_and_verts
codes = example_path.codes
self._paths = [_make_path(xy, codes,
Copy link
Contributor Author

@apaszke apaszke Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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...

internals_from=example_path,
unmask_verts=False)
Copy link
Member

Choose a reason for hiding this comment

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

unmask_verts is no longer there.

for xy in verts_pad]
return

self._paths = []
Expand Down
16 changes: 5 additions & 11 deletions lib/matplotlib/path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', '__weakref__')

code_type = np.uint8
vert_type = float

# Path codes
STOP = code_type(0) # 1 vertex
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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

pth._codes = codes
pth._readonly = False
if internals_from is not None:
Expand Down Expand Up @@ -269,16 +273,6 @@ def readonly(self):
"""
return self._readonly

def __copy__(self):
"""
Returns a shallow copy of the `Path`, which will share the
vertices and codes with the source `Path`.
"""
import copy
return copy.copy(self)

copy = __copy__

def __deepcopy__(self, memo=None):
"""
Returns a deepcopy of the `Path`. The `Path` will not be
Expand Down
6 changes: 0 additions & 6 deletions lib/matplotlib/tests/test_transforms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,12 +636,6 @@ def test_transformed_path():
[(0, 0), (r2, r2), (0, 2 * r2), (-r2, r2)],
atol=1e-15)

# Changing the path does not change the result (it's cached).
path.points = [(0, 0)] * 4
assert_allclose(trans_path.get_fully_transformed_path().vertices,
[(0, 0), (r2, r2), (0, 2 * r2), (-r2, r2)],
atol=1e-15)


def test_transformed_patch_path():
trans = mtransforms.Affine2D()
Expand Down