PERF: _api check function speedups#31071
PERF: _api check function speedups#31071scottshambaugh wants to merge 9 commits intomatplotlib:mainfrom
Conversation
|
A function named |
|
Random thought: The lists in class Axes:
_ORIENTATIONS = ValueGroup("orientation", ('horizontal', 'vertical'))
def bar(..., orentation=...):
self._ORENTATIONS.assert_contains(orientation)instead of def bar(..., orentation=...):
_api.check_in_list(("horizontal", "vertical"))("orientation", orientation)Whether Advantages:
Disadvantage:
|
|
That's a good point, renamed to For the lists of valid arguments, I'm so-so on a new class, but we should definitely consolidate the valid values anywhere that we reuse the same arguments in different places. Here's what I find:
|
|
Let's put the check_in_list alternative to the side for now. It was more a spark of an idea. But giben it seems not to be called too often, let's not completely reinvent checking. On the original proposal, I find Overall, I'm a bit unclear where the performance advantage comes from. The happy path is: matplotlib/lib/matplotlib/_api/__init__.py Lines 173 to 177 in 607d2c3 It seems there is not much to be gained from caching. How is the performance for is it almost as fast as Note: The multi-arg open came basically for free through the kwarg approach. But it's not something we really need or make substantial use of. Edit: AI tells me that |
|
I'm less and less convinced this change is worth it. Profiling at the whole draw cycle level with the script in #31001, I don't see an appreciable change in top-line percentage of call time spent in these functions (~0.8% of total time). The caching is added 20% overhead for each cache miss (ie the first time we call each function), which is partially offsetting the gains here. I think our time is better spent on other PRs. |
PR summary
This speeds up the
check_*functions in_api, largely by caching the function setup and deferring error handling.This necessarily comes with a big change in how these are called, as they now take in tuples and return functions. Before:
_api.check_shape((None, 2), arg=arg), after:_api.check_shape((None, 2))("arg", arg). This also means that we cannot check multiple args in one call, although this is not common in the codebase and the caching eliminates the overhead.check_shape- 3 us to 1 us (3x) - Cache and defer constructing the error messagecheck_isinstance- 2 us to 0.5 us (4x) - Cache and defer constructing the error messagecheck_in_list- 0.8 us to 0.5 us (1.5x) - Cast to tuple and cachecheck_getitem- 1 us to 1 us (1x) - No speedup, updated to match new APIIf there's a lot of pushback on the new API, only a few of these are on hot paths so I don't expect the overall benefit to be enormous. Each function has its own commit for review.
Inspired by @anntzer's comment here: #31001 (comment)
Profiling script I had Claude throw together:
Details
PR checklist