-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Entropy: improve performance by keeping colorgradient static. #32
Conversation
Hi Christoph, thank you for the feedback!
Maybe we could be even faster by computing (and caching) the actual colors for a set of (discretized) entropy values? We do something very similar in the Lines 77 to 81 in 4f995ef
Not yet, no. I knew I was asking for trouble though 😄. Running rendering and UI in separate threads is definitely the way to go. |
By the way: there is a ticket for slow entropy computation here: #7. I have a few ideas on how to speed it up, but most of this comes at the cost of accuracy (like computing only one entropy value per N byte chunk). |
Ah yes! I'll try that instead. That should be better. Although, performance wise it won't make as much of a difference (because with the current changes, all the compute is happening in the entropy calculation, I believe). But it is the cleaner approach (and probably just a tad faster).
PR #34 addresses this. While I think it makes the logic a bit more complicated and isn't in the spirit of "immediate mode" in egui, it is the approach suggested by the Pixel library (
In consequence, I would add multiple threads on-top of the "only paint if changed". But just using separate threads would also be appropriate, IMO. [I will cross-post this on #34.] |
I have implemented your suggestion of caching the discretized entropy value. I was not able to set up a benchmark (without a massive overhaul of the crate), and did not measure performance. Both implementations feel the same to me. I prefer the original version, since the logic is simpler. But feel free to decide which version you prefer and choose that one. |
Some performance measurements with tracing. All data is with the test file as supplied and Zoom set to 1x. And all other settings left at default. Of course, Entropy was selected. This is done on an M1 Macbook Air connected to a 5K monitor. NOTE: First measurements done running in debug 🤦🏼♂️. Left for posterity, but should not be considered. I am looking at the time of the
This is only possible due to the tracing support: #35 |
Thank you for the benchmark results and the update. Still need to figure out why it's slower when zoomed in. |
Thanks for the repo!
I ran
cargo flamegraph
on the code when using the Entropy visualization. This showed one very obvious area of improvement.csscolor::parse::parse
was parsing hex numbers as part of creating thecolograde::magma()
color gradient.I think the easiest solution is just to make
colorgrade::magma()
a static and re-use it. What do you think?Aside: Is there any work in making the UI into a separate thread from the calculations? The mouse-over is just sluggish when I change the color scheme.
Thanks for reviewing!