Make umfPoolCreateEx public

[igchor]

This function is useful in tests and simplifies integration with UR. Without it, the logic for provider lifetime management is duplicated in several places.

[vinser52]

This function is useful in tests and simplifies integration with UR. Without it, the logic for provider lifetime management is duplicated in several places.

As I understand in tests we are using C++ wrappers to automatically destroy emory provider instance. Can we use the same in UR?

[ldorau]

LGTM apart from one issue above to be fixed

[igchor]

This function is useful in tests and simplifies integration with UR. Without it, the logic for provider lifetime management is duplicated in several places.

As I understand in tests we are using C++ wrappers to automatically destroy emory provider instance. Can we use the same in UR?

In tests, we pretty much reimplement umfPoolCreateEx using C++ wrappers. In UR we can do the same but why reimplement it 2 times if we can just make this function public? Also, the wrappers itself would need to be reimplemented in UR since they are not a public API.

I think this function is useful in general, especially for applications that are in C (and cannot use unique_ptrs).

[vinser52]

In tests, we pretty much reimplement umfPoolCreateEx using C++ wrappers. In UR we can do the same but why reimplement it 2 times if we can just make this function public? Also, the wrappers itself would need to be reimplemented in UR since they are not a public API.

I think this function is useful in general, especially for applications that are in C (and cannot use unique_ptrs).

You got my point. We are using C++ in our tests, UR is also implemented in C++. The alternative approach could be to implement C++ wrappers for UMF (furthermore we have it mind anyway). I am not saying that I prefer this way or another, but let’s discuss it next week when I back from vacation. If we introduce new C API we have to guarantee backward compatibility and support it.

[igchor]

In tests, we pretty much reimplement umfPoolCreateEx using C++ wrappers. In UR we can do the same but why reimplement it 2 times if we can just make this function public? Also, the wrappers itself would need to be reimplemented in UR since they are not a public API.
I think this function is useful in general, especially for applications that are in C (and cannot use unique_ptrs).

You got my point. We are using C++ in our tests, UR is also implemented in C++. The alternative approach could be to implement C++ wrappers for UMF (furthermore we have it mind anyway). I am not saying that I prefer this way or another, but let’s discuss it next week when I back from vacation. If we introduce new C API we have to guarantee backward compatibility and support it.

Sure, let's do that.

For public C++ wrappers, what we were considering was a more high level interface which we could not use in UR I think (at least not entirely). The wrappers we have implemented right now, are mostly the things we have in umf_helpers.hpp (a way to create pool/provider from a C++ class with appropriate methods + factory functions returning unique_ptr) and I don't thing we want those to be public.

[igchor]

@vinser52 as discussed, here's the alternative approach : #126

[vinser52]

@vinser52 as discussed, here's the alternative approach : #126

I like #126 better than this PR. In #126 the signature looks more generic and extensible (by adding new control flags). Also with #126 we keep a single entry point to create a memory provider (umfMemoryProviderCreate) and I think it is clearer for the user.

So I vote for #126.
What do others think about it?

[bratpiorka]

@vinser52 as discussed, here's the alternative approach : #126

I like #126 better than this PR. In #126 the signature looks more generic and extensible (by adding new control flags). Also with #126 we keep a single entry point to create a memory provider (umfMemoryProviderCreate) and I think it is clearer for the user.

So I vote for #126. What do others think about it?

hm but what do you think of keeping only umfPoolCreateEx but named as umfPoolCreate? I mean we could have only the function with flags that could be 0 as default

[igchor]

@vinser52 as discussed, here's the alternative approach : #126

I like #126 better than this PR. In #126 the signature looks more generic and extensible (by adding new control flags). Also with #126 we keep a single entry point to create a memory provider (umfMemoryProviderCreate) and I think it is clearer for the user.
So I vote for #126. What do others think about it?

hm but what do you think of keeping only umfPoolCreateEx but named as umfPoolCreate? I mean we could have only the function with flags that could be 0 as default

Yeah, I think that would be pretty reasonable.

[vinser52]

hm but what do you think of keeping only umfPoolCreateEx but named as umfPoolCreate? I mean we could have only the function with flags that could be 0 as default

Yeah, I think that would be pretty reasonable.

I agree. it's a pity that the default arguments are not supported (without some tricks) in C

[ldorau]

@igchor Rebase please

[vinser52]

So, @igchor, will you extend the umfPoolCreate function with an additional flag instead of adding a new API?

[igchor]

So, @igchor, will you extend the umfPoolCreate function with an additional flag instead of adding a new API?

Yes, I will change #126