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

Fix for regression with colorbar limits #3874

Merged
merged 6 commits into from
Oct 12, 2021

Conversation

BioTurboNick
Copy link
Member

@BioTurboNick BioTurboNick commented Oct 10, 2021

#3839 solved a slowdown when re-rendering a plot for an animation, but reversed the benefits of #3838 for rendering the original plot.

This PR does two things:

  • Only updates the colorbars once when a plot is added. Previously it was running twice.
  • Stores the colorlimit values in a dedicated typed dictionary; storing them in the general properties dictionaries appeared to interfere with efficient code generation.

One area of concern: Are there other times when _update_sublot_args gets called outside of the pipeline, when the colorbar would need updating? If so, it would skip the colorbar updating routine.

Resolves #3873

@BioTurboNick
Copy link
Member Author

Though I'm still curious why all it caused a regression at all... with #3838, plotting took less than a second, but it's 3 seconds currently. Because all that first PR did was add type signatures.

@BioTurboNick
Copy link
Member Author

Got it down to 2 seconds by refactoring the if/else statement into functions.

@BioTurboNick
Copy link
Member Author

BioTurboNick commented Oct 11, 2021

I think the next frontier in performance improvement here is dealing with the fact that the clims calculation has to look at 4 different properties for each series. It doesn't seem to lead to any allocations but it does take a very long time run through them.

Especially annoying when they're all empty.

@BioTurboNick
Copy link
Member Author

BioTurboNick commented Oct 11, 2021

Alright! Unrolling that update loop got my big plot down to 1.5 s. Was 8 s before doing all of my recent profiling work.

Updating color bar limits is still a nontrivial amount of the computation time when profiling (50.1% of samples), but it's a lot less.

@BioTurboNick
Copy link
Member Author

BioTurboNick commented Oct 11, 2021

Tests are passing on my machine, so not sure what's happening here.

Oh, might just be related to the tick angle adjustments.

@BeastyBlacksmith BeastyBlacksmith merged commit d74ee63 into JuliaPlots:master Oct 12, 2021
isentropic pushed a commit to isentropic/Plots.jl that referenced this pull request Jan 16, 2022
* Fix for regression

* Remove call

* Refactored to dispatching

* Fixes

* Unrolling loop

* Change to IdDict in case objects mutated
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.

[BUG] Performance regression in colobar limits
2 participants