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

Separate PJ_CONTEXT from PJ #1047

Closed
bylee20 opened this issue Jun 19, 2018 · 3 comments
Closed

Separate PJ_CONTEXT from PJ #1047

bylee20 opened this issue Jun 19, 2018 · 3 comments

Comments

@bylee20
Copy link

bylee20 commented Jun 19, 2018

By using per-thread PJ_CONTEXT, one can use PROJ in thread safe way.
However, current implementation binds PJ_CONTEXT into PJ object, and this lowers the usability in multithreaded environment.

Since, for instance, proj_trans API uses PJ_CONTEXT in PJ object, one should have PJ object per thread as well as PJ_CONTEXT per thread.

To remove PJ_CONTEXT from PJ and make the APIs take PJ_CONTEXT as separated parameter will make the API more convenient.

@kbevers
Copy link
Member

kbevers commented Jun 19, 2018

I welcome any idea that can improve the situation with regards to multithreading. I don't understand exactly what it is you are proposing. Please elaborate with a few examples on how you want to change the API and the internal infrastructure. Keep in mind we need to keep backward compatibility with at least the proj_api.h API.

@bylee20
Copy link
Author

bylee20 commented Jun 20, 2018

Well... I doubt that it's possible to keep backward compatibility. I suppose to make PJ object thread-safe and all API takes PJ object should take separated PJ_CONTEXT parameter for multithreading.

For instance, with C++17 parallel for_each, current implementation requires PJ_CONTEXT object and PJ object per thread:

std::vector<PJ_COORD> coords = {...};
std::for_each(std::par_unseq, coords.begin(), coords.end(), [] (PJ_COORD &c) {
    thread_local PJ_CONTEXT *ctx = proj_context_create();
    thread_local PJ *pj = proj_create(ctx, "...");
    c = proj_trans(pj, PJ_FWD, c);
});

In this case, there's no point in having PJ_CONTEXT for each thread for thread safety because, eventually, you need PJ object per thread.

In order to make PJ_CONTEXT useful, I think PJ object should be thread-safe and anything thread-unsafe should be in PJ_CONTEXT:

std::vector<PJ_COORD> coords = {...};
PJ *pj = proj_create_mt("..."); // mt denotes multithread
std::for_each(std::par_unseq, coords.begin(), coords.end(), [pj] (PJ_COORD &c) {
    thread_local PJ_CONTEXT *ctx = proj_context_create();
    c = proj_trans_mt(ctx, pj, PJ_FWD, c); // use same PJ object for every thread
});

@kbevers
Copy link
Member

kbevers commented Jun 20, 2018

Well... I doubt that it's possible to keep backward compatibility.

That's the kicker. We can take this up again in a couple of years :-)

I agree that the concept of contexts is not particularly well thought out in PROJ. The lacking documentation on the subject isn't helping either. Unfortunately we can't change much at the moment without breaking the old API's.

The general idea with the contexts as they exists now is that you create one per thread. Within that thread several PJs can share the context. They are mostly useful for logging and coordinating reads from shared grid files.

I think it is extremely unlikely this will change in the foreseable future. While you proposition is a nice sentiment it is also very difficult to ensure that use of every possible PJ is safe for use across threads. Additionally it will require some serious structural work internally. I will close this for now but I am happy to discuss this again in the future when we have less legacy code to deal with.

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

No branches or pull requests

2 participants