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

Performance improvements #33

Merged
merged 3 commits into from
Apr 17, 2020
Merged

Performance improvements #33

merged 3 commits into from
Apr 17, 2020

Conversation

rafaqz
Copy link
Collaborator

@rafaqz rafaqz commented Apr 16, 2020

This PR fixes type stability issues, and uses StaticArrays for colorschemes. I'm getting over an order of magnitude performance improvement displaying live simulations with colorschemes.

@cormullion
Copy link
Member

cormullion commented Apr 16, 2020

Looks awesome!

Travis complains about this:

ERROR: LoadError: InitError: Evaluation into the closed module `ColorTypes` breaks incremental compilation because the side effects will not be permanent. This is likely due to some other module mutating `ColorTypes` with `include` during precompilation - don't do this.

Is this caused by something here?

Oh, this was probably caused by a recent PR in ColorTypes JuliaGraphics/ColorTypes.jl#178

@KristofferC
Copy link

What's the advantage of StaticArrays over using a tuple? Do you need to do math on the vector?

@rafaqz
Copy link
Collaborator Author

rafaqz commented Sep 10, 2020

I used static arrays so as to not change the interface too much in the PR, not knowing why they were vectors in the first place or what the other use cases are. I'm fine with them being tuples, StaticArrays.jl is a bit too heavy a dependency really.

@rafaqz
Copy link
Collaborator Author

rafaqz commented Sep 10, 2020

Is this for Gaston.jl load times? lets swap to tuples.

@KristofferC
Copy link

Is this for Gaston.jl load times?

Yes, at least that's why I looked at the load time for this package.

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

3 participants