improve performance of project_line_points #4601
Conversation
Benchmark ResultsSHA: 7863f3c1dc01fbd74edeb76b06e35ec48229f519 Warning These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking.
|
| space = (plot.space[])::Symbol | ||
| model = (plot.model[])::Mat4d | ||
| # Standard transform from input space to clip space | ||
| points = Makie.apply_transform(transform_func(plot), positions, space)::typeof(positions) |
There was a problem hiding this comment.
That's not guaranteed for 2D to 3D or 3D to 2D transformations...is this type annotation necessary here? Can we specify that it will be a Vector of VecT 8000 ypes maybe?
There was a problem hiding this comment.
This is required right now, since somehow it doesn't get inferred... This happens because transform_func doesn't infer. not sure what to do about it, I guess we could also put it behind a function barrier and extract transform_func before this function call and pass it as an argument.
There was a problem hiding this comment.
easiest would be to put everything after this behind another function barrier, apply_transform is already the function barrier for the dynamic transform_func result, but the stuff after is not guarded without ::typeof(positions)
|
I wonder if this has badly regressed at some point because I'm pretty sure I optimized the main drawing functions at some point so they didn't unnecessarily allocate. Could we add regression tests for this stuff? |
|
Well the benchmark seems to catch this really well... This may be the pr: |
|
I think it'd be nice to slowly add more benchmarks and whole flamegraphs, which should catch function regressions pretty well. |
|
@asinghvi17 ah sorry, I forgot to address your comment. |
PR:
12.143 ms (16 allocations: 49.59 MiB)
Master:
2.643 s (86997922 allocations: 2.02 GiB)