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 issue with poly! #4367

Open
3 tasks
oscarvdvelde opened this issue Sep 15, 2024 · 4 comments
Open
3 tasks

Performance issue with poly! #4367

oscarvdvelde opened this issue Sep 15, 2024 · 4 comments
Labels
bug dependencies Related to a package used by Makie GeometryBasics Performance also memory

Comments

@oscarvdvelde
Copy link

oscarvdvelde commented Sep 15, 2024

  • what version of Makie are you running? (]st -m Makie)
    Makie v0.21.10 on Julia 1.10.5
  • can you reproduce the bug with a fresh environment ? (]activate --temp; add Makie)
    not tried yet, but I did do a full ] update
  • What platform + GPU are you on?
    Windows 11, Intel and NVIDIA graphics (issue appears with both)

Today I tried to move from the use of GeoIO.jl + Meshes.viz! to draw a shapefile, to Shapefile.jl and Makie's own poly!
The former method had a very similar latency inducing bug earlier which was fixed last May: https://discourse.julialang.org/t/visualization-of-shapefile-meshes-jl-viz-very-slow

It turns out that using poly! is taking more than 3 seconds and makes my Observables (toggles, menus, clicks, zooms etc) very laggy right from the start, to the point of crashing (see below).
For now I will keep using my previous method, but I wanted to report this issue.

shapefile.zip

# with GeoIO and Meshes:

using GLMakie, GeoIO
using Meshes: viz!
fig = nothing; fig = Figure(size = (900,900))
ax1 = Axis(fig[1,1])
countries1 = GeoIO.load(raw"C:\yourpath\francespain.shp").geometry
@time p1 = viz!(ax1,countries1,segmentsize=0.75,showsegments=true,segmentcolor=:black,alpha=0.0)

0.193131 seconds (18.62 k allocations: 71.597 MiB)

# with Shapefile and Makie
using GLMakie, Shapefile
fig = nothing; fig = Figure(size = (900,900))
ax1 = Axis(fig[1,1])
countries2 = Shapefile.Handle(raw"C:\yourpath\francespain.shp").shapes
@time p2 = poly!(ax1, countries2; color=(:blue,0.0), strokewidth = 0.75, overdraw = true)
  
3.098870 seconds (4.13 M allocations: 215.289 MiB)

Additionally, the latter shows black contours instead of blue, but I may be doing it wrong.

An error of the renderloop resulting from poly!:

julia> Error in callback:
MethodError: no method matching draw_atomic(::GLMakie.Screen{GLFW.Window}, ::Scene, ::Poly{Tuple{Vector{GeometryBasics.MultiPolygon{2, Float64, GeometryBasics.Polygon{2, Float64, Point{2, Float64}, GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}, Vector{GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}}}, Vector{GeometryBasics.Polygon{2, Float64, Point{2, Float64}, GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}, Vector{GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}}}}}}}})

Closest candidates are:
draw_atomic(::GLMakie.Screen, ::Scene, ::Voxels)
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\drawing_primitives.jl:923
draw_atomic(::GLMakie.Screen, ::Scene, ::Volume)
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\drawing_primitives.jl:866
draw_atomic(::GLMakie.Screen, ::Scene, ::Surface)
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\drawing_primitives.jl:789
...

Stacktrace:
[1] insert!(screen::GLMakie.Screen{GLFW.Window}, scene::Scene, x::Plot)
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\drawing_primitives.jl:353
[2] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[3] invokelatest
@ .\essentials.jl:889 [inlined]
[4] push!(scene::Scene, plot::Plot)
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\scenes.jl:462
[5] plot!
@ C:\Users\oscar.julia\packages\Makie\WgbrE\src\interfaces.jl:410 [inlined]
[6] plot!(ax::Axis, plot::Poly{Tuple{Vector{GeometryBasics.MultiPolygon{2, Float64, GeometryBasics.Polygon{2, Float64, Point{2, Float64}, GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}, Vector{GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}}}, Vector{GeometryBasics.Polygon{2, Float64, Point{2, Float64}, GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}, Vector{GeometryBasics.LineString{2, Float64, Point{2, Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point{2, Float64}, Point{2, Float64}}, GeometryBasics.TupleView{Tuple{Point{2, Float64}, Point{2, Float64}}, 2, 1, Vector{Point{2, Float64}}}, false}}}}}}}}})
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\figureplotting.jl:412
[7] _create_plot!(::Function, ::Dict{Symbol, Any}, ::Axis, ::Vector{Union{Missing, Shapefile.Polygon}})
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\figureplotting.jl:381
[8] poly!(::Axis, ::Vararg{Any}; kw::@kwargs{color::Tuple{Symbol, Float64}, strokewidth::Float64, overdraw::Bool})
@ MakieCore C:\Users\oscar.julia\packages\MakieCore\k9SbO\src\recipes.jl:453
[9] (::var"#217#219")(sel::BitVector)
@ Main c:\Users\oscar\Documents\Work\Julia\LMA_visualizer.jl:762
[10] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[11] invokelatest
@ .\essentials.jl:889 [inlined]
[12] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[13] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[14] (::var"#223#224")(arg1#259::BitVector, arg2#260::BitVector, arg3#261::BitVector)
@ Main .\simdloop.jl:0
[15] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::@kwargs{})
@ Base .\essentials.jl:892
[16] invokelatest(::Any, ::Any, ::Vararg{Any})
@ Base .\essentials.jl:889
[17] (::Observables.MapCallback)(value::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:436
[18] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[19] invokelatest
@ .\essentials.jl:889 [inlined]
[20] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[21] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[22] (::var"#225#227")(tlims::GeometryBasics.HyperRectangle{2, Float64})
@ Main c:\Users\oscar\Documents\Work\Julia\LMA_visualizer.jl:919
[23] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[24] invokelatest
@ .\essentials.jl:889 [inlined]
[25] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[26] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[27] adjustlimits!(la::Axis)
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\blocks\axis.jl:990
[28] (::Makie.var"#1728#1759"{Axis})(pxa::GeometryBasics.HyperRectangle{2, Int64}, lims::GeometryBasics.HyperRectangle{2, Float64})
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\blocks\axis.jl:507
[29] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::@kwargs{})
@ Base .\essentials.jl:892
[30] invokelatest(::Any, ::Any, ::Vararg{Any})
@ Base .\essentials.jl:889
[31] (::Observables.OnAny)(value::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:420
[32] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[33] invokelatest
@ .\essentials.jl:889 [inlined]
[34] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[35] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[36] reset_limits!(ax::Axis; xauto::Bool, yauto::Bool, zauto::Bool)
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\blocks\axis.jl:640
[37] reset_limits!
@ C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\blocks\axis.jl:557 [inlined]
[38] plot!(ax::Axis, plot::Scatter{Tuple{Vector{Point{2, Float32}}}})
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\figureplotting.jl:421
[39] _create_plot!(::Function, ::Dict{Symbol, Any}, ::Axis, ::Vector{Point{2, Float32}})
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\figureplotting.jl:381
[40] scatter!(::Axis, ::Vararg{Any}; kw::@kwargs{marker::Symbol, markersize::Float64, strokecolor::Symbol, strokewidth::Float64, color::Vector{Float64}, colorrange::Tuple{Float64, Float64}, colormap::Symbol})
@ MakieCore C:\Users\oscar.julia\packages\MakieCore\k9SbO\src\recipes.jl:453
[41] (::var"#217#219")(sel::BitVector)
@ Main c:\Users\oscar\Documents\Work\Julia\LMA_visualizer.jl:807
[42] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[43] invokelatest
@ .\essentials.jl:889 [inlined]
[44] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[45] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[46] (::var"#223#224")(arg1#259::BitVector, arg2#260::BitVector, arg3#261::BitVector)
@ Main .\simdloop.jl:0
[47] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::@kwargs{})
@ Base .\essentials.jl:892
[48] invokelatest(::Any, ::Any, ::Vararg{Any})
@ Base .\essentials.jl:889
[49] (::Observables.MapCallback)(value::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:436
[50] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[51] invokelatest
@ .\essentials.jl:889 [inlined]
[52] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[53] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[54] (::var"#255#257")(click8::Int64)
@ Main c:\Users\oscar\Documents\Work\Julia\LMA_visualizer.jl:1336
[55] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[56] invokelatest
@ .\essentials.jl:889 [inlined]
[57] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[58] setindex!(observable::Observable, val::Any)
@ Observables C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123
[59] (::Makie.var"#2130#2140"{Button, Observable{Symbol}})(::MouseEvent)
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\blocks\button.jl:68
[60] (::Makie.var"#1515#1516"{Makie.var"#2130#2140"{Button, Observable{Symbol}}})(event::MouseEvent)
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\mousestatemachine.jl:94
[61] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[62] invokelatest
@ .\essentials.jl:889 [inlined]
[63] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[64] setindex!
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123 [inlined]
[65] (::Makie.var"#1585#1587"{Scene, Base.RefValue{Bool}, Base.RefValue{Union{Nothing, Makie.Mouse.Button}}, Base.RefValue{Float64}, Base.RefValue{Float64}, Base.RefValue{Bool}, Base.RefValue{Bool}, Base.RefValue{Union{Nothing, Makie.Mouse.Button}}, Base.RefValue{Bool}, Base.RefValue{Point{2, Float32}}, Base.RefValue{Point{2, Float64}}, Base.RefValue{Makie.Mouse.Action}, Observable{MouseEvent}, Float64, Module})(event::Makie.MouseButtonEvent)
@ Makie C:\Users\oscar.julia\packages\Makie\WgbrE\src\makielayout\mousestatemachine.jl:298
[66] #invokelatest#2
@ .\essentials.jl:892 [inlined]
[67] invokelatest
@ .\essentials.jl:889 [inlined]
[68] notify
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:206 [inlined]
[69] setindex!
@ C:\Users\oscar.julia\packages\Observables\YdEbO\src\Observables.jl:123 [inlined]
[70] (::GLMakie.var"#mousebuttons#167"{Observable{Makie.MouseButtonEvent}})(window::GLFW.Window, button::GLFW.MouseButton,
action::GLFW.Action, mods::Int32)
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\events.jl:104
[71] _MouseButtonCallbackWrapper(window::GLFW.Window, button::GLFW.MouseButton, action::GLFW.Action, mods::Int32)
@ GLFW C:\Users\oscar.julia\packages\GLFW\wmoTL\src\callback.jl:43
[72] PollEvents
@ C:\Users\oscar.julia\packages\GLFW\wmoTL\src\glfw3.jl:702 [inlined]
[73] pollevents(screen::GLMakie.Screen{GLFW.Window}, frame_state::Makie.TickState)
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\screen.jl:483
[74] on_demand_renderloop(screen::GLMakie.Screen{GLFW.Window})
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\screen.jl:979
[75] renderloop(screen::GLMakie.Screen{GLFW.Window})
@ GLMakie C:\Users\oscar.julia\packages\GLMakie\8uUlv\src\screen.jl:1007

@asinghvi17
Copy link
Member

asinghvi17 commented Sep 15, 2024

Huh, this could have multiple issues, but it might be an issue with an inefficiency in Shapefile.jl (what version of Shapefile.jl are you running?). Consider this addendum to your code:

julia> @time gb_geoms = GeoMakie.to_multipoly(countries2)
  2.376218 seconds (4.12 M allocations: 139.941 MiB, 0.69% gc time)
julia> @time p1 = poly!(ax1, gb_geoms; color = :transparent, strokewidth = 1, strokecolor = :blue)
  0.223099 seconds (103.81 k allocations: 66.597 MiB, 2.04% gc time, 24.87% compilation time)

Also, you should pass strokecolor in poly to set the size of your lines.

With GeoIO the conversion of Shapefile.jl geometry to Meshes.jl geometry is eager, with Shapefile+GeoInterface it's lazy, only converting in poly itself.

@ffreyer
Copy link
Collaborator

ffreyer commented Sep 15, 2024

On #4319 I'm getting:

fig = nothing; fig = Figure(size = (900,900))
ax1 = Axis(fig[1,1])
@time p2 = poly!(ax1, countries2; color=(:blue,0.0), strokewidth = 0.75, overdraw = true)
  0.421756 seconds (4.01 M allocations: 909.210 MiB, 9.01% gc time)

So maybe something that the refactor is fixing, even if it has gotten more memory hungry. (I guess GeometryBasics.merge(meshes) could use some optimization...)

Additionally, the latter shows black contours instead of blue, but I may be doing it wrong.

Yes, you are setting the mesh color to transparent blue. You should set strokecolor = :blue to get blue outlines.


After hunting down performance problems in merge(meshes):

@time p2 = poly!(ax1, countries2; color=(:blue,0.0), strokewidth = 0.75, overdraw = true)
  0.200260 seconds (1.39 M allocations: 111.060 MiB)

@ffreyer
Copy link
Collaborator

ffreyer commented Sep 16, 2024

JuliaGeometry/GeometryBasics.jl@5491a72

This is the other major source of allocations. With the runtime type, Julia ends up allocating for every point here, I believe. With a function barrier that goes away. There should be more cases like this across the other interface methods, but maybe that's something the GeoInterface people should look into. I think these issues would also go away if you had something like get_type(::Vector{Point{N, T)) where {N, T} = T in GeoInteface, for these geom types that are being passed around. Maybe that exists, or maybe it should?

After this change:

@time p2 = poly!(ax1, countries2; color=(:blue,0.0), strokewidth = 0.75, overdraw = true)
  0.124739 seconds (15.11 k allocations: 67.517 MiB)

@ffreyer ffreyer added GeometryBasics dependencies Related to a package used by Makie Performance also memory labels Sep 16, 2024
@asinghvi17
Copy link
Member

There's a proposal for that method in GeoInterface JuliaGeo/GeoInterface.jl#128, we should probably get around to writing that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dependencies Related to a package used by Makie GeometryBasics Performance also memory
Projects
None yet
Development

No branches or pull requests

3 participants