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

Efficient coordinate collection from LineStrings using reinterpret #32

Open
asinghvi17 opened this issue Apr 11, 2024 · 5 comments · May be fixed by #39
Open

Efficient coordinate collection from LineStrings using reinterpret #32

asinghvi17 opened this issue Apr 11, 2024 · 5 comments · May be fixed by #39

Comments

@asinghvi17
Copy link
Collaborator

import GeoFormatTypes as GFT, WellKnownGeometry as WKG, GeoInterface as GI

tups = tuple.(rand(300_000), rand(300_000))
geoms_as_points = GFT.val.(WKG.getwkb.(GI.Point.(tups)))
geoms_as_linestring = GFT.val(WKG.getwkb(GI.LineString(tups)))

# parse geoms as points, each WKB individually
@benchmark begin
    map($(geoms_as_points)) do geom
        only(reinterpret(Tuple{Float64, Float64}, view(geom, (5+1):length(geom)))) # this has to be parsed differently in EWKB and GeoPkg wkb
    end
end # median time 3ms v/s regular parsing 300ms

# parse the whole linestring
@benchmark collect(reinterpret(Tuple{Float64, Float64}, view($ls_as_wkb, (1+4+4+1):length($ls_as_wkb))))
# median time 290ns vs unknown (still running) 
# we could even drop collect here
@evetion
Copy link
Owner

evetion commented Oct 5, 2024

Fun thing, doing the above works fine, but ofcourse changes the return type to an NTuple. That sorta implies we got to change the return types of the other (point) methods too, which actually could slow down things:

function GI.getcoord(::GI.PointTrait, geom::WKBtype)
    offset = 1
    ncoord = GI.ncoord(geom)
    data = @view geom.val[headersize+offset:headersize+offset+sizeof(Float64)*ncoord-1]

    reinterpret(Float64, data)  # current 211.961 ns (2 allocations: 112 bytes)
    only(reinterpret(NTuple{ncoord,Float64}, data))  #   580.569 ns (7 allocations: 320 bytes)
    GC.@preserve geom unsafe_load(Ptr{NTuple{ncoord,Float64}}(pointer(geom.val, headersize + offset)))  # 466.837 ns (6 allocations: 224 bytes)
end

@rafaqz
Copy link
Collaborator

rafaqz commented Oct 5, 2024

Have you profiled this? That just looks like type instability to me

@evetion
Copy link
Owner

evetion commented Oct 6, 2024

Those three different variants were run using btime yes.

@rafaqz
Copy link
Collaborator

rafaqz commented Oct 6, 2024

No I mean ProfileView... Yours will have some red

@evetion evetion linked a pull request Oct 6, 2024 that will close this issue
@evetion
Copy link
Owner

evetion commented Oct 6, 2024

With the linked PR

import GeoFormatTypes as GFT, WellKnownGeometry as WKG, GeoInterface as GI

tups = tuple.(rand(300_000), rand(300_000))
geoms_as_linestring = WKG.getwkb(GI.LineString(tups))
@btime GI.coordinates(geoms_as_linestring);
98.625 μs (5 allocations: 4.58 MiB)

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 a pull request may close this issue.

3 participants