Performance issue with poly!

oscarvdvelde commented 1 month ago

[ ] 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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

https://github.com/JuliaGeometry/GeometryBasics.jl/pull/219/commits/5491a7290c9b3db295918f37f04b0408238f588e

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)

asinghvi17 commented 1 month ago

There's a proposal for that method in GeoInterface https://github.com/JuliaGeo/GeoInterface.jl/issues/128, we should probably get around to writing that...

oscarvdvelde commented 1 week ago

Using the current version of Makie and Shapefile, still on Julia 1.10.5:

  [e9467ef8] GLMakie v0.10.14
  [db073c08] GeoMakie v0.7.5
  [ee78f7c6] Makie v0.21.14
  [8e980c4a] Shapefile v0.13.1

using GLMakie
using Shapefile
fig = nothing; fig = Figure(size = (900,900))
ax1 = Axis(fig[1,1])
countries = Shapefile.Handle(raw"C:\Users\oscar\Documents\Work\Julia\LMA\map\francespain.shp").shapes
@time poly!(ax1, countries; strokecolor=:blue, strokewidth = 1.25, overdraw = true)
  3.227038 seconds (4.13 M allocations: 215.189 MiB, 1.72% gc time)

using GeoMakie
@time countries2 = GeoMakie.to_multipoly(countries);
  2.908216 seconds (4.12 M allocations: 139.934 MiB, 0.25% gc time)
 @time p2 = poly!(ax1, countries2; strokecolor=:blue, strokewidth = 1.25, overdraw = true)
  0.236980 seconds (9.56 k allocations: 75.267 MiB, 10.12% gc time)

So, the situation is as in @asinghvi17 's comment, and using GeoMakie.to_multipoly is fine. But the further speedup mentioned in the one before last comment has not arrived yet.

(I came back to this after getting a crash in my backup solution, GeoIO + Meshes: viz! It turns out that GeoIO was also preventing my update of many packages, so I'd rather not use it now)

pkg> add GeoIO
   Resolving package versions...
⌅ [927a84f5] ↓ DelaunayTriangulation v1.6.1 ⇒ v0.8.12
⌃ [e9467ef8] ↓ GLMakie v0.10.14 ⇒ v0.8.12
⌃ [f5a160d5] + GeoIO v1.0.0
⌅ [61d90e0f] ↓ GeoJSON v0.8.1 ⇒ v0.7.3
⌃ [db073c08] ↓ GeoMakie v0.7.5 ⇒ v0.5.1
⌅ [5c1252a2] ↓ GeometryBasics v0.4.11 ⇒ v0.4.10
⌅ [3955a311] ↓ GridLayoutBase v0.11.0 ⇒ v0.9.2
⌅ [ee78f7c6] ↓ Makie v0.21.14 ⇒ v0.19.12
⌅ [20f20a25] ↓ MakieCore v0.8.9 ⇒ v0.6.9
⌅ [0a4f8689] ↓ MathTeXEngine v0.6.1 ⇒ v0.5.7
⌅ [eacbb407] + Meshes v0.34.12
⌅ [8e980c4a] ↓ Shapefile v0.13.1 ⇒ v0.10.0

MakieOrg / Makie.jl

Performance issue with poly! #4367