Skip to content

Thread-safe reproject with ThreadFunctors #288

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

Merged
merged 26 commits into from
Apr 16, 2025
Merged

Thread-safe reproject with ThreadFunctors #288

merged 26 commits into from
Apr 16, 2025

Conversation

rafaqz
Copy link
Member

@rafaqz rafaqz commented Apr 10, 2025

Havent run this yet, but should be close to working.

@asinghvi17 turns out it needs 2 layers of functors to deal with is3d

Discussed in #287

Changelog:

  • Refactor the apply pipeline to get rid of closures in favour of Applicators, which are able to run apply on arrays, geoms and featurecollections.
  • Add a TaskFunctors struct that allows one to use individual functors per task. This allows thread safety for reentrant C APIs.
  • Refactor reproject, in the Proj backend, to use this functionality

@asinghvi17
Copy link
Member

Lol - I had a very similar approach, including an equivalent of the ApplyToN wrapper, but using TaskLocalValues.jl. But if we never reuse the value in a task then we don't need that package.

My struct was a bit different:

struct KnownInputDimTransformation{Z, M, T}
    transf::T
end

function KnownInputDimTransformation{Z, M}(transf::T) where {Z, M, T}
    KnownInputDimTransformation{Z, M, T}(transf)
end

function (t::KnownInputDimTransformation{Z, M, T})(p) where {Z, M, T}
    if Z
        return t.transf(GI.x(p), GI.y(p), GI.z(p))
    else
        return t.transf(GI.x(p), GI.y(p))
    end
end

currently does not handle Z and M but could be trivially extended to handle those and add default values if we want.

@asinghvi17
Copy link
Member

Boom

julia> @be GO.reproject($mpolys, $trans)
Benchmark: 13 samples with 1 evaluation
 min    7.543 ms (75012 allocs: 3.300 MiB)
 median 7.617 ms (75012 allocs: 3.300 MiB)
 mean   7.975 ms (75012 allocs: 3.300 MiB, 2.72% gc time)
 max    11.873 ms (75012 allocs: 3.300 MiB, 35.34% gc time)

julia> @be GO.reproject($mpolys, $trans; threaded = true)
Benchmark: 45 samples with 1 evaluation
 min    1.934 ms (75275 allocs: 3.496 MiB)
 median 1.970 ms (75275 allocs: 3.496 MiB)
 mean   2.229 ms (75275 allocs: 3.496 MiB, 5.59% gc time)
 max    6.006 ms (75275 allocs: 3.496 MiB, 65.74% gc time)

asinghvi17 and others added 7 commits April 10, 2025 17:44
ProjString needs +type=crs but regular String does not.

This is likely because I stopped converting everything to String and started relying on the Proj constructors.
@asinghvi17 asinghvi17 merged commit aef4c07 into main Apr 16, 2025
8 checks passed
asinghvi17 referenced this pull request Apr 17, 2025
* using GOCore

* Fix errors found in docs

* Set the current module in the Literate source

This way, docstrings are resolved respective to where the page is.

* add TGGeometry to docs project

* Update docs/src/api.md
@asinghvi17 asinghvi17 deleted the threadfunctors branch May 15, 2025 01:24
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 this pull request may close these issues.

2 participants