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

Speed up setting up of visuals #10006

Merged
merged 2 commits into from May 11, 2020
Merged

Speed up setting up of visuals #10006

merged 2 commits into from May 11, 2020

Conversation

mattpap
Copy link
Contributor

@mattpap mattpap commented May 11, 2020

Based on a suggestion in another PR (I can't find it right now), I checked what's the performance impact of all those comparisons in core/visuals.ts, and there is a visible negative impact for large datasets. I tested in Chrome and FF. For example, plotting/file/color_catter with N=100_000 has its run time reduced by roughly 100ms (700->600ms in Chrome and 1100->1000 in FF). There's quite a bit of dispersion though (not unexpected with JIT and GC), so I started working on benchmarking tooling for bokehjs, so that we could get consistent and statistically significant results. Though it will take awhile to finish this.

@@ -316,16 +304,14 @@ export class Fill extends ContextProperties {

protected _set_vectorize(ctx: Context2d, i: number): void {
this.cache_select("fill_color", i)
if (ctx.fillStyle !== this.cache.fill_color)
ctx.fillStyle = this.cache.fill_color
ctx.fillStyle = this.cache.fill_color
Copy link
Member

@bryevdv bryevdv May 11, 2020

Choose a reason for hiding this comment

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

eventually, something this would probably be ideal to work towards:

ctx.fillStyle = this.cache.fill_color(i)

to remove all the clunky explicit cache_select calls

Maybe there is an even better way to handle the scalar/vector split even more efficiently so that if fill_color="red" there is truly only one call and one test during rendering.

Copy link
Contributor Author

@mattpap mattpap May 11, 2020

Choose a reason for hiding this comment

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

I started gearing towards a cleaner separation between scalars and vectors in the last commit of PR #10004. I will address this in near future.

@bryevdv bryevdv merged commit d547416 into master May 11, 2020
23 checks passed
@bryevdv bryevdv deleted the mattpap/visuals_speedup branch May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants