Skip to content

Speed up Collection.set_paths#29398

Merged
QuLogic merged 3 commits intomatplotlib:mainfrom
scottshambaugh:3d_speedups_2
Jan 30, 2025
Merged

Speed up Collection.set_paths#29398
QuLogic merged 3 commits intomatplotlib:mainfrom
scottshambaugh:3d_speedups_2

Conversation

@scottshambaugh
Copy link
Contributor

@scottshambaugh scottshambaugh commented Jan 3, 2025 •
edited
Loading

PR summary

This builds on #16793 to implement __slots__ on Path objects, and to improve a lighter weight creation path. Relative to that MR, it does not modify the garbage collection on Paths, and adds a smattering of minor 3D rendering improvements. For a 200x200 plot_surface test case, I am seeing a 1.3x speedup after averaging over a few profiler runs, though note that this gives better performance over a much wider scope as well.

Relevant issue which this won't fully close: #16659

import time
import numpy as np
import matplotlib.pyplot as plt

fig = plt.figure()
ax = fig.add_subplot(111, projection='3d')

x = y = np.linspace(-1, 1, 200)
X, Y = np.meshgrid(x, y)
Z = X ** 2 + Y ** 2

print("Timing...")

start_time = time.perf_counter()
ax.plot_surface(X, Y, Z, rstride=1, cstride=1)
fig.canvas.draw()
end_time = time.perf_counter()

plt.close()
print(f"Time taken: {end_time - start_time:.4f} seconds")

PR checklist

@scottshambaugh scottshambaugh added this to the v3.11.0 milestone Jan 3, 2025
@scottshambaugh scottshambaugh marked this pull request as draft January 3, 2025 05:58
oscargus
oscargus reviewed Jan 3, 2025
View reviewed changes
scottshambaugh
scottshambaugh commented Jan 3, 2025
View reviewed changes
@scottshambaugh scottshambaugh marked this pull request as ready for review January 7, 2025 18:59
@scottshambaugh scottshambaugh mentioned this pull request Jan 7, 2025
Closed
6 tasks
@scottshambaugh scottshambaugh force-pushed the 3d_speedups_2 branch 3 times, most recently from 3831f2e to 1b256c2 Compare January 8, 2025 18:23
@timhoffm
Copy link
Member

timhoffm commented Jan 8, 2025

How much of the speed up is coming from slots? I would have anticipated that attribute lookups on paths are negligible compared to other operations, and thus the speed up is small. In that case, I would not go for slots as the impose certain restrictions that could become obstacles.

@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Jan 9, 2025 •
edited
Loading

Not too much is attributable to __slots__ as far as I can tell... the run-to-run profiler variation is high enough that I'm not confident in any changes under 10%. I'm not actually how that dunder would show up in a profiler run, but I'm not seeing its line number in the Path init calls. I was basing making the change on the parent PR, but if we want the flexibility of keeping __dict__ that makes sense to me. Removed.

image

tacaswell
tacaswell reviewed Jan 9, 2025
View reviewed changes
@tacaswell
Copy link
Member

tacaswell commented Jan 9, 2025

To not have a third round, the test that is dropped is not testing anything because path.points is not a known attribute and was causing issues with __slots__.

@tacaswell
Copy link
Member

tacaswell commented Jan 9, 2025

I would not expect to see __slots__ show up in a profiler as it controls how the pyobjects in c are created (and if they have a __dict__) and would expect memory reduction rather than runtime reduction.

@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Jan 24, 2025

Rebased to main, this is good to go.

timhoffm
timhoffm approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Minor suggestions

@scottshambaugh
Faster Path creation
bcca610
@scottshambaugh
3d calculation speedups
240c041 
_to_unmasked_float_array updates

Linting and tweaks

mean zsort

small perf gain

linting

Fix tests

Linting and tweaks

mean zsort

small perf gain

linting

Fix tests

Update lib/matplotlib/collections.py

Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>

remove Path __slots__

Code review suggestions
@scottshambaugh
Remove test
c6b9214
@QuLogic QuLogic merged commit 4cc7f94 into matplotlib:main Jan 30, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell left review comments

@oscargus oscargus oscargus left review comments

@QuLogic QuLogic QuLogic approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees

No one assigned

Labels

Performance topic: collections and mappables topic: mplot3d topic: path handling

Projects

None yet

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

Speeding up Axes3D.plot_surface 4-8x

5 participants

@scottshambaugh @timhoffm @tacaswell @QuLogic @oscargus