Various cleanups to backends code. #8711
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed some debug statements and old comments; whitespace / line wrapping. In particular, debug comments which simply indicate that a function has been called can easily be replaced by tools such as https://pypi.python.org/pypi/hunter or https://pypi.python.org/pypi/smiley.
tacaswell
reviewed
Jun 9, 2017
| else: self.canvas._tkcanvas.delete(self.lastrect) | ||
| y0 = height - y0 | ||
| y1 = height - y1 | ||
| if hasattr(self, "lastrect"): |
There was a problem hiding this comment.
This is a hot-path for UI, Under normal usage I think we expect this to exist for often that in does not so the try..except pattern is marginally faster.
def tester(obj):
if hasattr(obj, 'string'):
pass
def tryer(obj):
try:
obj.string
except:
pass
class A:
def __init__(self):
self.string = 'foo'
class B:
...In [61]: %timeit tester(1)
459 ns ± 3.11 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [62]: %timeit tryer(1)
524 ns ± 0.955 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [63]:
In [64]: a = A()
In [65]: %timeit tryer(a)
123 ns ± 0.202 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
In [66]: %timeit tester(a)
194 ns ± 0.281 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
In [67]:
In [68]: b = B()
In [69]: %timeit tryer(b)
488 ns ± 1.11 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [70]: %timeit tester(b)
423 ns ± 2.14 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [74]: There was a problem hiding this comment.
The following patch
diff --git a/lib/matplotlib/backends/backend_tkagg.py b/lib/matplotlib/backends/backend_tkagg.py
index af0ed62f9..4d212efe8 100644
--- a/lib/matplotlib/backends/backend_tkagg.py
+++ b/lib/matplotlib/backends/backend_tkagg.py
@@ -736,7 +736,14 @@ class NavigationToolbar2TkAgg(NavigationToolbar2, Tk.Frame):
try: self.lastrect
except AttributeError: pass
else: self.canvas._tkcanvas.delete(self.lastrect)
+ from time import perf_counter
+ start = perf_counter()
+ elapsed_dummy = perf_counter() - start
+ start = perf_counter()
self.lastrect = self.canvas._tkcanvas.create_rectangle(x0, y0, x1, y1)
+ elapsed_draw = perf_counter() - start
+ elapsed_corrected = elapsed_draw - elapsed_dummy
+ print(f"create_rectangle took {elapsed_corrected * 1e6}us")
shows that the call to create_rectangle that's just below takes ~100-300us, so I think worrying about 50ns for the sake of clarity is totally overkill.
|
👍 modulo the hot-path concern. |
tacaswell
approved these changes
Jun 9, 2017
|
The check is not the slowest thing in this code path, so 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removed some debug statements and old comments; whitespace / line
wrapping (to 79 characters).
(FWIW the debug code in backend_gtkcairo is apparently broken -- it occasionally refers to a function
fn_namewhich doesn't exist (it's_fn_name))PR Summary
PR Checklist