Skip to content
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.

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

Small optimizations #64

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Small optimizations #64

wants to merge 4 commits into from

Conversation

tgbrooks
Copy link

This is a couple of fairly small optimizations. I'm fairly tentative about these, since I'm scared it might break something. In particular, caching the outer_bounds and outer_position properties seems risky. These all work well for me, but more testing would be good.

Reducing the amount of accesses to layout_needed might seem unimportant, and it might be, but in the well log plots this is being accessed several thousand times a second and takes about ~1% of the run time. Note how every single draw(), _draw(), _draw_dispatch(), and do_layout() call all check this property, and each time it is accessed, the property goes through all components in the container recursively. So if the hierarchy of containers got any deeper (it's only about 3 in my case), the number of checks could get quite ridiculous. This removes a few of them, but doesn't fix the overarching problem.

intersect_bounds, too, gets called surprisingly often and was very inefficient before since it would convert to a different representation of rectangles, intersect those, and then convert back.

The outer_bounds and outer_positions are accessed a lot (for is_in checks, largely) and accesses a bunch of other properties. This could take a couple percent of the runtime in my use cases. I'm worried that I haven't covered all the cases for the depends_on property and could risk breaking existing code this way.

Thoughts?

@tgbrooks
Copy link
Author

tgbrooks commented Aug 9, 2012

I no longer see much reason to use this. The performance gains really seem quite negligible. Still, this highlights some of the inefficiencies of enable and some of these might be worth elaborating on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant