Re: RFC: Add C Bindings to Geode Native Client

[Blake Bender]

Hello everyone,

I neglected to put an end date on this RFC, but it's been open for 2+ weeks
now, and I believe we're close to (at?) consensus, so I would like to close
it out and move on ASAP.  If you still have anything urgent to add, please
reply here and let's hash it out.  All other things being equal, I'll
summarize anything left that needs it and move the RFC to 'Active' status
Monday morning (PDT).

Thanks,

Blake

Re: RFC: Add C Bindings to Geode Native Client

[Blake Bender]

Jake, to follow up on your previous comment re: consensus, after
face-to-face conversation I believe the following is our current status.

Agreed:
* Strong types with opaque struct pointers
* No complete structs in the API/ABI
* split shared libraries - one for C, one for C++, one mixed-mode assembly
for .net Framework
* We will prevent exceptions across the interface boundary.
* Namespacing/prefixing.  I've included an example of this in the updated
RFC.
* Pattern and naming conventions for 'new', 'delete', and class methods.
Also provided an example in RFC doc
* Strong types with opaque struct pointers
* No complete structs in the API/ABI

This leaves us with just the serialization left undefined.  I've added a
note about the general approach we intend to take, but a lot of the detail
will need to be determined when we get into the work itself, and I consider
it beyond the scope of this RFC.

Updated RFC is at
https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
.

Thanks,

Blake


On Wed, Apr 1, 2020 at 7:06 PM Jacob Barrett  wrote:

> Agreed. It was sort of an inside joke. There used to be a ccache
> executable but that was deleted a long time ago. I am in no way advocating
> for ccache as the directory or library name.
>
> > On Apr 1, 2020, at 2:48 PM, Robert Houghton 
> wrote:
> >
> > Quick note: ccache is a C/C++ compiler cache. Examples using 'ccache' as
> > the name are confusing.
> >
> >> On Tue, Mar 31, 2020 at 3:56 PM Jacob Barrett 
> wrote:
> >>
> >>
> >>
>  On Mar 31, 2020, at 3:06 PM, Blake Bender  wrote:
> >>>
> >>> We in this instance means the native client team.  As far as specific
> >>> comments, I'm going to suggest we not go down that road, because this
> >> feels
> >>> a little more adversarial to me than it needs to be already.
> >>
> >> Sorry it feels adversarial. From below I think there is a
> misunderstanding
> >> of my preferences.
> >>
> >>> Suffice to
> >>> say that from my own perspective, in both what you wrote and what I got
> >>> from our in-person conversation on Monday, your answer to the general
> >>> question "Should the C bindings be part of the native client?" appeared
> >> to
> >>> be no, thus a separate repository seemed a perfectly reasonable
> >>> assumption.  I had hoped to do this in the geode-native repo to begin
> >> with,
> >>> so your assent to this makes life easier.
> >>
> >> This may be the point of confusion because I have never intended to give
> >> the impression that the C-bindings should be separate from the
> geode-native
> >> repo. My examples even integrate it with the geode-native project. I do
> >> believe it should be separate sources and separate includes. I would not
> >> want to be doing a C++ project and have C functions clouding my IDE or
> >> vice versa. Perhaps that is where the confusion comes from.
> >>
> >>> As far as my concerns about inefficiency, what I meant is essentially
> yes
> >>> we have multiple copies of the same code in the release, and this
> always
> >>> makes me a little uneasy.  I've seen a lot of compatibility bugs in my
> >>> career having to do with different products having different versions
> of
> >>> the same code, so my natural inclination is to avoid it when possible.
> >>> Having both C++ and C-style functions exported from the same library
> >>> doesn't give me any heartburn at all, so simply compiling the C
> bindings
> >>> into the existing shared library just means one less copy of the code
> in
> >>> the installation.  I fear I am in the minority in this opinion,
> however,
> >>> and it's not something I'm really doctrinaire about, so I'll defer.
> >> I would really like to understand your concerns but I don’t understand
> how
> >> combining the symbols into a single file resolves the versioning issue?
> Can
> >> your help me understand what the different produces with different
> versions
> >> means and how it would apply to this case?
> >>
> >> If the C and C++ symbols are both exported from the same shared library
> >> would you have a common include directory as well or would you spit the
> >> includes? I could live with a combine library but not a combined include
> >> headers.
> >>
> >>> So are we good here?  I'd like to get the RFC wrapped up and move on to
> >>> building this.
> >>
> >> Do you feel there is a consensus? I feel like there is a lot left that
> >> isn’t either in the original RFC, hasn’t been discussed here or is
> still a
> >> sticking point. You could update the RFC with what we have discussed and
> >> see if consensus is reached.
> >>
> >> Sticking points:
> >> * Single or split shared libraries
> >> * Single or split includes
> >> * Single or split source repository
> >>
> >> Undefined:
> >> * Exception handling (I gave one example but didn’t get feedback or
> >> consensus)
> >> * Namespacing/Prefixing
> >> * Pattern and naming conventions for new, delete and class methods.
> >> * Handling of (de)serialization hand off or callbacks int

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

Agreed. It was sort of an inside joke. There used to be a ccache executable but 
that was deleted a long time ago. I am in no way advocating for ccache as the 
directory or library name.

> On Apr 1, 2020, at 2:48 PM, Robert Houghton  wrote:
> 
> Quick note: ccache is a C/C++ compiler cache. Examples using 'ccache' as
> the name are confusing.
> 
>> On Tue, Mar 31, 2020 at 3:56 PM Jacob Barrett  wrote:
>> 
>> 
>> 
 On Mar 31, 2020, at 3:06 PM, Blake Bender  wrote:
>>> 
>>> We in this instance means the native client team.  As far as specific
>>> comments, I'm going to suggest we not go down that road, because this
>> feels
>>> a little more adversarial to me than it needs to be already.
>> 
>> Sorry it feels adversarial. From below I think there is a misunderstanding
>> of my preferences.
>> 
>>> Suffice to
>>> say that from my own perspective, in both what you wrote and what I got
>>> from our in-person conversation on Monday, your answer to the general
>>> question "Should the C bindings be part of the native client?" appeared
>> to
>>> be no, thus a separate repository seemed a perfectly reasonable
>>> assumption.  I had hoped to do this in the geode-native repo to begin
>> with,
>>> so your assent to this makes life easier.
>> 
>> This may be the point of confusion because I have never intended to give
>> the impression that the C-bindings should be separate from the geode-native
>> repo. My examples even integrate it with the geode-native project. I do
>> believe it should be separate sources and separate includes. I would not
>> want to be doing a C++ project and have C functions clouding my IDE or
>> vice versa. Perhaps that is where the confusion comes from.
>> 
>>> As far as my concerns about inefficiency, what I meant is essentially yes
>>> we have multiple copies of the same code in the release, and this always
>>> makes me a little uneasy.  I've seen a lot of compatibility bugs in my
>>> career having to do with different products having different versions of
>>> the same code, so my natural inclination is to avoid it when possible.
>>> Having both C++ and C-style functions exported from the same library
>>> doesn't give me any heartburn at all, so simply compiling the C bindings
>>> into the existing shared library just means one less copy of the code in
>>> the installation.  I fear I am in the minority in this opinion, however,
>>> and it's not something I'm really doctrinaire about, so I'll defer.
>> I would really like to understand your concerns but I don’t understand how
>> combining the symbols into a single file resolves the versioning issue? Can
>> your help me understand what the different produces with different versions
>> means and how it would apply to this case?
>> 
>> If the C and C++ symbols are both exported from the same shared library
>> would you have a common include directory as well or would you spit the
>> includes? I could live with a combine library but not a combined include
>> headers.
>> 
>>> So are we good here?  I'd like to get the RFC wrapped up and move on to
>>> building this.
>> 
>> Do you feel there is a consensus? I feel like there is a lot left that
>> isn’t either in the original RFC, hasn’t been discussed here or is still a
>> sticking point. You could update the RFC with what we have discussed and
>> see if consensus is reached.
>> 
>> Sticking points:
>> * Single or split shared libraries
>> * Single or split includes
>> * Single or split source repository
>> 
>> Undefined:
>> * Exception handling (I gave one example but didn’t get feedback or
>> consensus)
>> * Namespacing/Prefixing
>> * Pattern and naming conventions for new, delete and class methods.
>> * Handling of (de)serialization hand off or callbacks into non-C code
>> 
>> Agreed:
>> * Strong types with opaque struct pointers
>> * No complete structs in the API/ABI
>> 
>> -Jake
>> 
>> 

Re: RFC: Add C Bindings to Geode Native Client

[Robert Houghton]

Quick note: ccache is a C/C++ compiler cache. Examples using 'ccache' as
the name are confusing.

On Tue, Mar 31, 2020 at 3:56 PM Jacob Barrett  wrote:

>
>
> > On Mar 31, 2020, at 3:06 PM, Blake Bender  wrote:
> >
> > We in this instance means the native client team.  As far as specific
> > comments, I'm going to suggest we not go down that road, because this
> feels
> > a little more adversarial to me than it needs to be already.
>
> Sorry it feels adversarial. From below I think there is a misunderstanding
> of my preferences.
>
> >  Suffice to
> > say that from my own perspective, in both what you wrote and what I got
> > from our in-person conversation on Monday, your answer to the general
> > question "Should the C bindings be part of the native client?" appeared
> to
> > be no, thus a separate repository seemed a perfectly reasonable
> > assumption.  I had hoped to do this in the geode-native repo to begin
> with,
> > so your assent to this makes life easier.
>
> This may be the point of confusion because I have never intended to give
> the impression that the C-bindings should be separate from the geode-native
> repo. My examples even integrate it with the geode-native project. I do
> believe it should be separate sources and separate includes. I would not
> want to be doing a C++ project and have C functions clouding my IDE or
> vice versa. Perhaps that is where the confusion comes from.
>
> > As far as my concerns about inefficiency, what I meant is essentially yes
> > we have multiple copies of the same code in the release, and this always
> > makes me a little uneasy.  I've seen a lot of compatibility bugs in my
> > career having to do with different products having different versions of
> > the same code, so my natural inclination is to avoid it when possible.
> > Having both C++ and C-style functions exported from the same library
> > doesn't give me any heartburn at all, so simply compiling the C bindings
> > into the existing shared library just means one less copy of the code in
> > the installation.  I fear I am in the minority in this opinion, however,
> > and it's not something I'm really doctrinaire about, so I'll defer.
> I would really like to understand your concerns but I don’t understand how
> combining the symbols into a single file resolves the versioning issue? Can
> your help me understand what the different produces with different versions
> means and how it would apply to this case?
>
> If the C and C++ symbols are both exported from the same shared library
> would you have a common include directory as well or would you spit the
> includes? I could live with a combine library but not a combined include
> headers.
>
> > So are we good here?  I'd like to get the RFC wrapped up and move on to
> > building this.
>
> Do you feel there is a consensus? I feel like there is a lot left that
> isn’t either in the original RFC, hasn’t been discussed here or is still a
> sticking point. You could update the RFC with what we have discussed and
> see if consensus is reached.
>
> Sticking points:
> * Single or split shared libraries
> * Single or split includes
> * Single or split source repository
>
> Undefined:
> * Exception handling (I gave one example but didn’t get feedback or
> consensus)
> * Namespacing/Prefixing
> * Pattern and naming conventions for new, delete and class methods.
> * Handling of (de)serialization hand off or callbacks into non-C code
>
> Agreed:
> * Strong types with opaque struct pointers
> * No complete structs in the API/ABI
>
> -Jake
>
>

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

> On Mar 31, 2020, at 3:06 PM, Blake Bender  wrote:
> 
> We in this instance means the native client team.  As far as specific
> comments, I'm going to suggest we not go down that road, because this feels
> a little more adversarial to me than it needs to be already.

Sorry it feels adversarial. From below I think there is a misunderstanding of 
my preferences.

>  Suffice to
> say that from my own perspective, in both what you wrote and what I got
> from our in-person conversation on Monday, your answer to the general
> question "Should the C bindings be part of the native client?" appeared to
> be no, thus a separate repository seemed a perfectly reasonable
> assumption.  I had hoped to do this in the geode-native repo to begin with,
> so your assent to this makes life easier.

This may be the point of confusion because I have never intended to give the 
impression that the C-bindings should be separate from the geode-native repo. 
My examples even integrate it with the geode-native project. I do believe it 
should be separate sources and separate includes. I would not want to be doing 
a C++ project and have C functions clouding my IDE or  vice versa. Perhaps that 
is where the confusion comes from. 

> As far as my concerns about inefficiency, what I meant is essentially yes
> we have multiple copies of the same code in the release, and this always
> makes me a little uneasy.  I've seen a lot of compatibility bugs in my
> career having to do with different products having different versions of
> the same code, so my natural inclination is to avoid it when possible.
> Having both C++ and C-style functions exported from the same library
> doesn't give me any heartburn at all, so simply compiling the C bindings
> into the existing shared library just means one less copy of the code in
> the installation.  I fear I am in the minority in this opinion, however,
> and it's not something I'm really doctrinaire about, so I'll defer.
I would really like to understand your concerns but I don’t understand how 
combining the symbols into a single file resolves the versioning issue? Can 
your help me understand what the different produces with different versions 
means and how it would apply to this case?

If the C and C++ symbols are both exported from the same shared library would 
you have a common include directory as well or would you spit the includes? I 
could live with a combine library but not a combined include headers.

> So are we good here?  I'd like to get the RFC wrapped up and move on to
> building this.

Do you feel there is a consensus? I feel like there is a lot left that isn’t 
either in the original RFC, hasn’t been discussed here or is still a sticking 
point. You could update the RFC with what we have discussed and see if 
consensus is reached.

Sticking points:
* Single or split shared libraries
* Single or split includes
* Single or split source repository

Undefined:
* Exception handling (I gave one example but didn’t get feedback or consensus)
* Namespacing/Prefixing
* Pattern and naming conventions for new, delete and class methods.
* Handling of (de)serialization hand off or callbacks into non-C code

Agreed:
* Strong types with opaque struct pointers
* No complete structs in the API/ABI

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Blake Bender]

We in this instance means the native client team.  As far as specific
comments, I'm going to suggest we not go down that road, because this feels
a little more adversarial to me than it needs to be already.  Suffice to
say that from my own perspective, in both what you wrote and what I got
from our in-person conversation on Monday, your answer to the general
question "Should the C bindings be part of the native client?" appeared to
be no, thus a separate repository seemed a perfectly reasonable
assumption.  I had hoped to do this in the geode-native repo to begin with,
so your assent to this makes life easier.

As far as my concerns about inefficiency, what I meant is essentially yes
we have multiple copies of the same code in the release, and this always
makes me a little uneasy.  I've seen a lot of compatibility bugs in my
career having to do with different products having different versions of
the same code, so my natural inclination is to avoid it when possible.
Having both C++ and C-style functions exported from the same library
doesn't give me any heartburn at all, so simply compiling the C bindings
into the existing shared library just means one less copy of the code in
the installation.  I fear I am in the minority in this opinion, however,
and it's not something I'm really doctrinaire about, so I'll defer.

So are we good here?  I'd like to get the RFC wrapped up and move on to
building this.

Thanks,

Blake


On Tue, Mar 31, 2020 at 2:03 PM Jacob Barrett  wrote:

>
>
> > On Mar 31, 2020, at 1:48 PM, Matthew Reddington 
> wrote:
> >
> > A separate repo is our interpretation of the comments generated by this
> RFC.
>
> Can you please quote specific statements that you interpreted to suggest
> separate repositories. I would like to understand where this interpretation
> comes from.
>
> > We consider these separate projects and that they should be organized as
> such.
>
> Who is we? Does the we reflect the owners of the comments you are
> referencing above?
>
> In the end Geode is a single project in the eyes of ASF.
>
> -Jake
>
>

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

> On Mar 31, 2020, at 1:48 PM, Matthew Reddington  
> wrote:
> 
> A separate repo is our interpretation of the comments generated by this RFC.

Can you please quote specific statements that you interpreted to suggest 
separate repositories. I would like to understand where this interpretation 
comes from. 

> We consider these separate projects and that they should be organized as such.

Who is we? Does the we reflect the owners of the comments you are referencing 
above?

In the end Geode is a single project in the eyes of ASF.

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

> On Mar 31, 2020, at 12:25 PM, Blake Bender  wrote:
> 
> Just want to make sure I understand what you're after here.  We should have
> a "ccache" directory or similar in the geode-native repo, where we build C
> bindings for the client, then we should compile them into a shared library
> containing all of the code, and export/make visible only the C interface?

Correct.

> So the geode native installer will contain shared libraries representing 3
> copies of all the code, with the static library in the build tree making a
> total of 4?

Let me see if I am counting the same things you are:
Out of the existing geode-native repo:
1 - C++ dynamic library (only C++ symbols exported)
2 - C dynamic library (only C symbols export)
3 - .NET Framework mixed mode assembly (deprecated, only .NET symbols exported)

Out of the new geode-net repo: (keeping it short, dropping the dot)
4 - .NET Core mixed mode assembly
or
4 - .NET Core assembly with dependency on C dynamic library

This also supposes that we have one installer for all this, maybe we have 
multiple installers, maybe we select what we want to install, maybe it doesn’t 
matter because in the end Geode doesn’t even produce an installer.

> I'm starting to be concerned with the overall inefficiency of this plan.
> Is this the best we can do?

Can you describe the inefficiency you are thinking about. 

I see having the C dynamic library statically linked the C++ bits as a much 
smaller footprint to whatever language it is eventually bound to. All the C++ 
templated symbols that are unused by the C binding layer should be left our by 
the linker. I won’t have to copy two dynamic libraries with my C-binding based 
application. Think about .NET core, if we can’t do mixed mode assemblies then 
it must be dynamically dependent on the C dynamic library, if the C library was 
dynamic to the C++ then we have to have that library also in the path. Recall 
how hard it is for some to manage having .NET and SSLimpl in the same path, now 
it's worse. If the inefficiency is disk space, that’s cheeper than support time 
on getting library paths correct.

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Matthew Reddington]

A separate repo is our interpretation of the comments generated by this RFC. 
It’s easier to combine repositories later than it is to take them apart. The 
take away of this library is the ABI; that it will presently link against the 
C++ library is an implementation detail. We consider these separate projects 
and that they should be organized as such.

> On Mar 31, 2020, at 12:25 PM, Blake Bender  wrote:
> 
> Just want to make sure I understand what you're after here.  We should have
> a "ccache" directory or similar in the geode-native repo, where we build C
> bindings for the client, then we should compile them into a shared library
> containing all of the code, and export/make visible only the C interface?
> So the geode native installer will contain shared libraries representing 3
> copies of all the code, with the static library in the build tree making a
> total of 4?
> 
> I'm starting to be concerned with the overall inefficiency of this plan.
> Is this the best we can do?
> 
> Thanks,
> 
> Blake
> 
> 
> On Tue, Mar 31, 2020 at 12:05 PM Jacob Barrett  wrote:
> 
>> Given that the C-binding will be tightly coupled with the C++ layer and
>> written in C++ I don’t think it make sense to have its own repository. In
>> order for the C-binding need to access internal, non-exported methods, for
>> things like serialization it will need to be statically linked to the C++
>> library. If we use separate repositories and builds then we need to build a
>> static library for the dependency in the C-binding library. This seems
>> overly complicated to me for little to no gain. Can you please elaborate on
>> the the pros of having a separate repository for the C-binding?
>> 
>> For sure agree on the .net core repo. Tough depending on the mixed mode
>> binary support we still might run into the dependency issues on static
>> library.
>> 
>> -Jake
>> 
>> 
>>> On Mar 31, 2020, at 11:23 AM, Matthew Reddington 
>> wrote:
>>> 
>>> I would like to request the addition of two new repositories under
>> Apache in order to implement this RFC and to take advantage of it in
>> practice. That would be Apache/geode-c-client and
>> Apache/geode-dot-net-core-client.
>>> 
 On Mar 30, 2020, at 3:33 PM, Jacob Barrett  wrote:
 
 
>> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
>> <
>> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
>>> 
 
 Quick stab at a POC for thread exception handling.
 
 -Jake
 
>>> 
>> 
>> 

Re: RFC: Add C Bindings to Geode Native Client

[Dan Smith]

Once we do have agreement on what new repositories we want, I think any pmc
member should be able to create them on gitbox.apache.org. Deleting them if
we decide we don't want them is harder :)

-Dan

On Tue, Mar 31, 2020, 12:26 PM Blake Bender  wrote:

> Just want to make sure I understand what you're after here.  We should have
> a "ccache" directory or similar in the geode-native repo, where we build C
> bindings for the client, then we should compile them into a shared library
> containing all of the code, and export/make visible only the C interface?
> So the geode native installer will contain shared libraries representing 3
> copies of all the code, with the static library in the build tree making a
> total of 4?
>
> I'm starting to be concerned with the overall inefficiency of this plan.
>  Is this the best we can do?
>
> Thanks,
>
> Blake
>
>
> On Tue, Mar 31, 2020 at 12:05 PM Jacob Barrett 
> wrote:
>
> > Given that the C-binding will be tightly coupled with the C++ layer and
> > written in C++ I don’t think it make sense to have its own repository. In
> > order for the C-binding need to access internal, non-exported methods,
> for
> > things like serialization it will need to be statically linked to the C++
> > library. If we use separate repositories and builds then we need to
> build a
> > static library for the dependency in the C-binding library. This seems
> > overly complicated to me for little to no gain. Can you please elaborate
> on
> > the the pros of having a separate repository for the C-binding?
> >
> > For sure agree on the .net core repo. Tough depending on the mixed mode
> > binary support we still might run into the dependency issues on static
> > library.
> >
> > -Jake
> >
> >
> > > On Mar 31, 2020, at 11:23 AM, Matthew Reddington <
> mredding...@pivotal.io>
> > wrote:
> > >
> > > I would like to request the addition of two new repositories under
> > Apache in order to implement this RFC and to take advantage of it in
> > practice. That would be Apache/geode-c-client and
> > Apache/geode-dot-net-core-client.
> > >
> > >> On Mar 30, 2020, at 3:33 PM, Jacob Barrett 
> wrote:
> > >>
> > >>
> >
> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
> > <
> >
> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
> > >
> > >>
> > >> Quick stab at a POC for thread exception handling.
> > >>
> > >> -Jake
> > >>
> > >
> >
> >
>

Re: RFC: Add C Bindings to Geode Native Client

[Blake Bender]

Just want to make sure I understand what you're after here.  We should have
a "ccache" directory or similar in the geode-native repo, where we build C
bindings for the client, then we should compile them into a shared library
containing all of the code, and export/make visible only the C interface?
So the geode native installer will contain shared libraries representing 3
copies of all the code, with the static library in the build tree making a
total of 4?

I'm starting to be concerned with the overall inefficiency of this plan.
 Is this the best we can do?

Thanks,

Blake


On Tue, Mar 31, 2020 at 12:05 PM Jacob Barrett  wrote:

> Given that the C-binding will be tightly coupled with the C++ layer and
> written in C++ I don’t think it make sense to have its own repository. In
> order for the C-binding need to access internal, non-exported methods, for
> things like serialization it will need to be statically linked to the C++
> library. If we use separate repositories and builds then we need to build a
> static library for the dependency in the C-binding library. This seems
> overly complicated to me for little to no gain. Can you please elaborate on
> the the pros of having a separate repository for the C-binding?
>
> For sure agree on the .net core repo. Tough depending on the mixed mode
> binary support we still might run into the dependency issues on static
> library.
>
> -Jake
>
>
> > On Mar 31, 2020, at 11:23 AM, Matthew Reddington 
> wrote:
> >
> > I would like to request the addition of two new repositories under
> Apache in order to implement this RFC and to take advantage of it in
> practice. That would be Apache/geode-c-client and
> Apache/geode-dot-net-core-client.
> >
> >> On Mar 30, 2020, at 3:33 PM, Jacob Barrett  wrote:
> >>
> >>
> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
> <
> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
> >
> >>
> >> Quick stab at a POC for thread exception handling.
> >>
> >> -Jake
> >>
> >
>
>

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

Given that the C-binding will be tightly coupled with the C++ layer and written 
in C++ I don’t think it make sense to have its own repository. In order for the 
C-binding need to access internal, non-exported methods, for things like 
serialization it will need to be statically linked to the C++ library. If we 
use separate repositories and builds then we need to build a static library for 
the dependency in the C-binding library. This seems overly complicated to me 
for little to no gain. Can you please elaborate on the the pros of having a 
separate repository for the C-binding?

For sure agree on the .net core repo. Tough depending on the mixed mode binary 
support we still might run into the dependency issues on static library. 

-Jake


> On Mar 31, 2020, at 11:23 AM, Matthew Reddington  
> wrote:
> 
> I would like to request the addition of two new repositories under Apache in 
> order to implement this RFC and to take advantage of it in practice. That 
> would be Apache/geode-c-client and Apache/geode-dot-net-core-client.
> 
>> On Mar 30, 2020, at 3:33 PM, Jacob Barrett  wrote:
>> 
>> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
>>  
>> 
>> 
>> Quick stab at a POC for thread exception handling.
>> 
>> -Jake
>> 
> 

Re: RFC: Add C Bindings to Geode Native Client

[Matthew Reddington]

I would like to request the addition of two new repositories under Apache in 
order to implement this RFC and to take advantage of it in practice. That would 
be Apache/geode-c-client and Apache/geode-dot-net-core-client.

> On Mar 30, 2020, at 3:33 PM, Jacob Barrett  wrote:
> 
> https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
>  
> 
> 
> Quick stab at a POC for thread exception handling.
> 
> -Jake
> 

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

https://github.com/pivotal-jbarrett/geode-native/tree/ee34cfbb5bddb55f5f890bb013c75d7780a787ae/ccache
 


Quick stab at a POC for thread exception handling.

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

> On Mar 30, 2020, at 10:32 AM, Blake Bender  wrote:
> 
> Just want to +1 on the use of a dynamic library - this really has to be a
> shared lib, interop with most other languages demands it.  On the other
> hand, I'm not a huge fan of making this a separate library from the native
> client itself, simply because proliferation of binary files makes life
> difficult for our users.  native client on Windows already uses a
> mixed-mode assembly to avoid having separate C++ and .net DLLs, *and* we're
> potentially lugging around cryptoimpl with us if someone wishes to use
> SSL.  Adding another library just for C bindings makes for a clean
> separation of the APIs, but I'm not certain how much value this adds, and
> it definitely adds complexity to any installation/setup.

To be clear I don’t think Mathew or I are suggesting the the C++ dynamic 
library be dynamically linked to the C dynamic library so that your language 
binding or application have to dynamically load both libraries. I would suggest 
that the C library statically link the C++ library and hide the symbols from 
export. This prevents users from seeing or using anything we haven’t written a 
C wrapper for. I think statically linking to the C++ library is going to be 
required anyway in order for the C library to use non-exported internals to 
address issues like serialization between languages. For example, region 
methods that take raw byte* that contain the serialized form of the data. These 
methods wouldn’t be exposed via the C++ library exports.

The other rational for keeping them separate is that I wouldn’t want to mix the 
C and C++ headers into my application. The C and C++ gods will clash and all 
hell will break loose.

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

> On Mar 30, 2020, at 2:38 PM, Matthew Reddington  
> wrote:
> 
 Does it make sense to have the CacheFactory concept at all (especially
> since it is more of a builder)? Could we have some C struct that can be
> used to create the cache, where the struct has fields for all the
> configuration? In general can we rethink the API so that it makes sense for
> C or other language bindings?
>>> * I recommend we avoid introducing real types in the exported interface.
> In order to support future revision, we’d have to introduce version and
> size information into the struct a la THE WIN32 API. Remember that? Having
> to zero out the struct then setting the size and version members before the
> other members? This is why they did that.
>> What is this in reference to? Using structs to hold the pointers? I
> already over that solution for just using opaque struct pointers.
> 
> Jake, you mentioned passing structs to initialize the cache instead of
> using a cache factory. I'm leery of introducing additional concrete types
> to yet another API. Since this isn't where we are slow, flexibility and
> future proofing are preferred. This is something the team is going to have
> to look at more options, good examples, and decide what we're willing to
> deal with long term.

Oh, yeah, that makes total sense dealing configuration structs. I am cool with 
the factory/builder pattern. Its just a ton of functions. ;)


>>> * This library needs a thread safe means of error handling. There is not
> enough fleshed out in this RFC to have a meaningful conversation about this
> at this time.
>> Can you elaborate on this? If the functions all returned some error code
> what thread safety issues are you thinking about?
> 
> When I think of C and error handling, I think of errno, which isn't thread
> safe. I don't want to see a repeat of that. I suppose it will boil down to
> output parameters and returning error codes.

Yeah, errno is a huge mistake.

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Matthew Reddington]

>>>Does it make sense to have the CacheFactory concept at all (especially
since it is more of a builder)? Could we have some C struct that can be
used to create the cache, where the struct has fields for all the
configuration? In general can we rethink the API so that it makes sense for
C or other language bindings?
>> * I recommend we avoid introducing real types in the exported interface.
In order to support future revision, we’d have to introduce version and
size information into the struct a la THE WIN32 API. Remember that? Having
to zero out the struct then setting the size and version members before the
other members? This is why they did that.
> What is this in reference to? Using structs to hold the pointers? I
already over that solution for just using opaque struct pointers.

Jake, you mentioned passing structs to initialize the cache instead of
using a cache factory. I'm leery of introducing additional concrete types
to yet another API. Since this isn't where we are slow, flexibility and
future proofing are preferred. This is something the team is going to have
to look at more options, good examples, and decide what we're willing to
deal with long term.

>> * This library needs a thread safe means of error handling. There is not
enough fleshed out in this RFC to have a meaningful conversation about this
at this time.
> Can you elaborate on this? If the functions all returned some error code
what thread safety issues are you thinking about?

When I think of C and error handling, I think of errno, which isn't thread
safe. I don't want to see a repeat of that. I suppose it will boil down to
output parameters and returning error codes.

On Mon, Mar 30, 2020 at 10:39 AM Blake Bender  wrote:

> Just want to +1 on the use of a dynamic library - this really has to be a
> shared lib, interop with most other languages demands it.  On the other
> hand, I'm not a huge fan of making this a separate library from the native
> client itself, simply because proliferation of binary files makes life
> difficult for our users.  native client on Windows already uses a
> mixed-mode assembly to avoid having separate C++ and .net DLLs, *and* we're
> potentially lugging around cryptoimpl with us if someone wishes to use
> SSL.  Adding another library just for C bindings makes for a clean
> separation of the APIs, but I'm not certain how much value this adds, and
> it definitely adds complexity to any installation/setup.
>
> I've experienced firsthand the problems of exceptions thrown (or attempting
> to be thrown, anyway) across the interface boundary.  It doesn't work, to
> put it mildly, so for sure we'll have to address the issue.
>
> I've updated the doc to reflect the latest thinking based on this
> conversation.  Let me know if you still think we need more.
>
> Thanks,
>
> Blake
>
>
> On Mon, Mar 30, 2020 at 7:01 AM Jacob Barrett  wrote:
>
> >
> >
> > > On Mar 27, 2020, at 4:04 PM, Matthew Reddington <
> mredding...@pivotal.io>
> > wrote:
> > > * C does not have namespaces or overloading, so we will need a naming
> > convention to differentiate our types and methods from any other library
> or
> > the application code. That means all types and functions should be
> > prepended with “geode_” or something similar.
> >
> > Ha… I knew there was something on my list I forgot to mention. Yes, all
> > methods need to be prefixed. I recall there being something around Apache
> > branding that required us to use apache::geode:: as our base namespace
> > rather than just geode::. Same would hold true for this prefix also
> then. I
> > would suggest apache_geode_X. The method names get pretty long so it may
> be
> > worth double checking that geode_ is sufficient.
> >
> > > * We must absolutely produce a dynamic library. Not all FFI’s support
> > static linking.
> >
> > Yeah, I have changed my mind on that. Ideally we would produce both but
> > dynamic only is fine for now. Given the source release nature of Apache
> > anyway this doesn’t really matter. Anyone could pull the source and
> build a
> > static lib as needed.
> >
> > > * I recommend we avoid introducing real types in the exported
> interface.
> > In order to support future revision, we’d have to introduce version and
> > size information into the struct a la THE WIN32 API. Remember that?
> Having
> > to zero out the struct then setting the size and version members before
> the
> > other members? This is why they did that.
> >
> > What is this in reference to? Using structs to hold the pointers? I
> > already over that solution for just using opaque struct pointers.
> >
> > > * The implementation needs to guard against throwing exceptions across
> > the library boundary.
> >
> > I was wondering about this. It feels like all these wrappers should catch
> > exceptions and convert them to a known set of int error codes, which are
> > either returned or passed by reference on the functions. None of that
> makes
> > for really clean readable C code but then again its C.

Re: RFC: Add C Bindings to Geode Native Client

[Blake Bender]

Just want to +1 on the use of a dynamic library - this really has to be a
shared lib, interop with most other languages demands it.  On the other
hand, I'm not a huge fan of making this a separate library from the native
client itself, simply because proliferation of binary files makes life
difficult for our users.  native client on Windows already uses a
mixed-mode assembly to avoid having separate C++ and .net DLLs, *and* we're
potentially lugging around cryptoimpl with us if someone wishes to use
SSL.  Adding another library just for C bindings makes for a clean
separation of the APIs, but I'm not certain how much value this adds, and
it definitely adds complexity to any installation/setup.

I've experienced firsthand the problems of exceptions thrown (or attempting
to be thrown, anyway) across the interface boundary.  It doesn't work, to
put it mildly, so for sure we'll have to address the issue.

I've updated the doc to reflect the latest thinking based on this
conversation.  Let me know if you still think we need more.

Thanks,

Blake


On Mon, Mar 30, 2020 at 7:01 AM Jacob Barrett  wrote:

>
>
> > On Mar 27, 2020, at 4:04 PM, Matthew Reddington 
> wrote:
> > * C does not have namespaces or overloading, so we will need a naming
> convention to differentiate our types and methods from any other library or
> the application code. That means all types and functions should be
> prepended with “geode_” or something similar.
>
> Ha… I knew there was something on my list I forgot to mention. Yes, all
> methods need to be prefixed. I recall there being something around Apache
> branding that required us to use apache::geode:: as our base namespace
> rather than just geode::. Same would hold true for this prefix also then. I
> would suggest apache_geode_X. The method names get pretty long so it may be
> worth double checking that geode_ is sufficient.
>
> > * We must absolutely produce a dynamic library. Not all FFI’s support
> static linking.
>
> Yeah, I have changed my mind on that. Ideally we would produce both but
> dynamic only is fine for now. Given the source release nature of Apache
> anyway this doesn’t really matter. Anyone could pull the source and build a
> static lib as needed.
>
> > * I recommend we avoid introducing real types in the exported interface.
> In order to support future revision, we’d have to introduce version and
> size information into the struct a la THE WIN32 API. Remember that? Having
> to zero out the struct then setting the size and version members before the
> other members? This is why they did that.
>
> What is this in reference to? Using structs to hold the pointers? I
> already over that solution for just using opaque struct pointers.
>
> > * The implementation needs to guard against throwing exceptions across
> the library boundary.
>
> I was wondering about this. It feels like all these wrappers should catch
> exceptions and convert them to a known set of int error codes, which are
> either returned or passed by reference on the functions. None of that makes
> for really clean readable C code but then again its C.
>
> > * This library needs a thread safe means of error handling. There is not
> enough fleshed out in this RFC to have a meaningful conversation about this
> at this time.
>
> Can you elaborate on this? If the functions all returned some error code
> what thread safety issues are you thinking about? If we need to get more
> detail from the exception back to the caller we could use thread local
> storage for message string, stack, etc. Perhaps taking some lessons from
> JNI and having a few exception checking/clearing/description methods with
> thread safe storage.
>
> > * I would like to see an ability for the client to specify their own
> allocators. This would require an overhaul of the existing C++ ABI and may
> have an impact on our dependencies. This is another talk for a more
> complete RFC.
>
> I think this is out of scope for sure. As you state the current C++ API
> lacks this ability as well as all the internals. Future us issues.
>
> -Jake
>
>

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

> On Mar 27, 2020, at 4:04 PM, Matthew Reddington  
> wrote:
> * C does not have namespaces or overloading, so we will need a naming 
> convention to differentiate our types and methods from any other library or 
> the application code. That means all types and functions should be prepended 
> with “geode_” or something similar.

Ha… I knew there was something on my list I forgot to mention. Yes, all methods 
need to be prefixed. I recall there being something around Apache branding that 
required us to use apache::geode:: as our base namespace rather than just 
geode::. Same would hold true for this prefix also then. I would suggest 
apache_geode_X. The method names get pretty long so it may be worth double 
checking that geode_ is sufficient.

> * We must absolutely produce a dynamic library. Not all FFI’s support static 
> linking. 

Yeah, I have changed my mind on that. Ideally we would produce both but dynamic 
only is fine for now. Given the source release nature of Apache anyway this 
doesn’t really matter. Anyone could pull the source and build a static lib as 
needed.

> * I recommend we avoid introducing real types in the exported interface. In 
> order to support future revision, we’d have to introduce version and size 
> information into the struct a la THE WIN32 API. Remember that? Having to zero 
> out the struct then setting the size and version members before the other 
> members? This is why they did that.

What is this in reference to? Using structs to hold the pointers? I already 
over that solution for just using opaque struct pointers.

> * The implementation needs to guard against throwing exceptions across the 
> library boundary.

I was wondering about this. It feels like all these wrappers should catch 
exceptions and convert them to a known set of int error codes, which are either 
returned or passed by reference on the functions. None of that makes for really 
clean readable C code but then again its C. 

> * This library needs a thread safe means of error handling. There is not 
> enough fleshed out in this RFC to have a meaningful conversation about this 
> at this time.

Can you elaborate on this? If the functions all returned some error code what 
thread safety issues are you thinking about? If we need to get more detail from 
the exception back to the caller we could use thread local storage for message 
string, stack, etc. Perhaps taking some lessons from JNI and having a few 
exception checking/clearing/description methods with thread safe storage.

> * I would like to see an ability for the client to specify their own 
> allocators. This would require an overhaul of the existing C++ ABI and may 
> have an impact on our dependencies. This is another talk for a more complete 
> RFC.

I think this is out of scope for sure. As you state the current C++ API lacks 
this ability as well as all the internals. Future us issues.

-Jake

Re: RFC: Add C Bindings to Geode Native Client

[Matthew Reddington]

* As per my offline comment and as Jake had pointed out, you can cast pointers 
to incomplete types, this will leverage the C type system better than mere 
typedefs, and enforce a modicum of an interface’s contract.
* C does not have namespaces or overloading, so we will need a naming 
convention to differentiate our types and methods from any other library or the 
application code. That means all types and functions should be prepended with 
“geode_” or something similar.
* We must absolutely produce a dynamic library. Not all FFI’s support static 
linking. This will also give us greater control over the symbols being 
exported. I agree that this dynamic link target should not share it’s symbol 
space with the C++ library. This grants much greater flexibility 
* I recommend we avoid introducing real types in the exported interface. In 
order to support future revision, we’d have to introduce version and size 
information into the struct a la THE WIN32 API. Remember that? Having to zero 
out the struct then setting the size and version members before the other 
members? This is why they did that.
* The implementation needs to guard against throwing exceptions across the 
library boundary.
* This library needs a thread safe means of error handling. There is not enough 
fleshed out in this RFC to have a meaningful conversation about this at this 
time.
* I would like to see an ability for the client to specify their own 
allocators. This would require an overhaul of the existing C++ ABI and may have 
an impact on our dependencies. This is another talk for a more complete RFC.

> On Mar 27, 2020, at 8:52 AM, Jacob Barrett  wrote:
> 
> Another couple of thing I thought of last night. 
> 
> The RFC should probably include mentions of tools like SWIG or Clang 
> libtooling for auto generating C-bindings for C++ headers.
> 
> One important area that isn’t covered is how this would address serialization 
> of types/objects in the bound language. For example, if I used this to bind 
> to Go how would my Go object be passed to put, get, or any other method that 
> takes a serializable object? When received how is this language going to be 
> invoked to de-serialize? More time needs to be spent on this aspect of the 
> interface.
> 
> -Jake
> 
> 
>> On Mar 26, 2020, at 3:10 PM, Jacob Barrett  wrote:
>> 
>> Forgot to include the source example: 
>> https://github.com/pivotal-jbarrett/geode-native/tree/e47698bad3cf725dcbddaad458813e26e75c8c71/ccache
>>  
>> 
>> 
>> 
>>> On Mar 26, 2020, at 2:56 PM, Jacob Barrett >> > wrote:
>>> 
>>> On the strongly typed, I found a solution I like even better.
>>> 
>>> struct CacheFactory;
>>> typedef struct CacheFactory CacheFactory;
>>> 
>>> struct Cache;
>>> typedef struct Cache Cache;
>>> 
>>> CacheFactory* createCacheFactory() {
>>>  return reinterpret_cast(
>>>  new apache::geode::client::CacheFactory());
>>> }
>>> 
>>> void destroyCacheFactory(CacheFactory* cacheFactory) {
>>>  delete 
>>> reinterpret_cast(cacheFactory);
>>> }
>>> 
>>> Cache* createCache(CacheFactory* cacheFactory) {
>>>  return reinterpret_cast(new apache::geode::client::Cache(
>>>  reinterpret_cast(cacheFactory)
>>>  ->create()));
>>> }
>>> 
>>> void destroyCache(Cache* cache) {
>>>  delete reinterpret_cast(cache);
>>> }
>>> 
>>> The code below is type safe and won’t compile.
>>> CacheFactory cacheFactory = createCacheFactory();
>>> destroyCache(cacheFactory);
>>> 
>>> 
>>> -Jake
>>> 
>>> 
 On Mar 26, 2020, at 12:22 PM, Jacob Barrett >>> > wrote:
 
 I am onboard with the idea but I would like more details in the RFC.
 
 I would prefer that the C bindings be in its own library. The only symbols 
 that should be visible from this library should be the C exported symbols, 
 the internal C++ symbols should be hidden.
 
 I feel like this library makes the most sense as a static library. I saw 
 this because in .NET we would link it as a mixed mode assembly to avoid 
 having multiple shared libraries to link. If we did a node client I would 
 expect it too would statically link this library as into its shared 
 library. Same I imagine would apply to all other language bindings.
 
 How are you planning on managing allocation and deallocation of these 
 opaque void* pointers. You vaguely showed the allocation but deallocation 
 would be a good example. I would assume some “destroy” method for every 
 “create” method.
 
 Does it make sense to have the CacheFactory concept at all (especially 
 since it is more of a builder)? Could we have some C struct that can be 
 used to create the cache, where the struct has fields for all the 
 configuration? In general can we rethink the API so that it makes sense 
 for C or other language bindings?
 
 What

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

Another couple of thing I thought of last night. 

The RFC should probably include mentions of tools like SWIG or Clang libtooling 
for auto generating C-bindings for C++ headers.

One important area that isn’t covered is how this would address serialization 
of types/objects in the bound language. For example, if I used this to bind to 
Go how would my Go object be passed to put, get, or any other method that takes 
a serializable object? When received how is this language going to be invoked 
to de-serialize? More time needs to be spent on this aspect of the interface.

-Jake


> On Mar 26, 2020, at 3:10 PM, Jacob Barrett  wrote:
> 
> Forgot to include the source example: 
> https://github.com/pivotal-jbarrett/geode-native/tree/e47698bad3cf725dcbddaad458813e26e75c8c71/ccache
>  
> 
> 
> 
>> On Mar 26, 2020, at 2:56 PM, Jacob Barrett > > wrote:
>> 
>> On the strongly typed, I found a solution I like even better.
>> 
>> struct CacheFactory;
>> typedef struct CacheFactory CacheFactory;
>> 
>> struct Cache;
>> typedef struct Cache Cache;
>> 
>> CacheFactory* createCacheFactory() {
>>   return reinterpret_cast(
>>   new apache::geode::client::CacheFactory());
>> }
>> 
>> void destroyCacheFactory(CacheFactory* cacheFactory) {
>>   delete 
>> reinterpret_cast(cacheFactory);
>> }
>> 
>> Cache* createCache(CacheFactory* cacheFactory) {
>>   return reinterpret_cast(new apache::geode::client::Cache(
>>   reinterpret_cast(cacheFactory)
>>   ->create()));
>> }
>> 
>> void destroyCache(Cache* cache) {
>>   delete reinterpret_cast(cache);
>> }
>> 
>> The code below is type safe and won’t compile.
>> CacheFactory cacheFactory = createCacheFactory();
>> destroyCache(cacheFactory);
>> 
>> 
>> -Jake
>> 
>> 
>>> On Mar 26, 2020, at 12:22 PM, Jacob Barrett >> > wrote:
>>> 
>>> I am onboard with the idea but I would like more details in the RFC.
>>> 
>>> I would prefer that the C bindings be in its own library. The only symbols 
>>> that should be visible from this library should be the C exported symbols, 
>>> the internal C++ symbols should be hidden.
>>> 
>>> I feel like this library makes the most sense as a static library. I saw 
>>> this because in .NET we would link it as a mixed mode assembly to avoid 
>>> having multiple shared libraries to link. If we did a node client I would 
>>> expect it too would statically link this library as into its shared 
>>> library. Same I imagine would apply to all other language bindings.
>>> 
>>> How are you planning on managing allocation and deallocation of these 
>>> opaque void* pointers. You vaguely showed the allocation but deallocation 
>>> would be a good example. I would assume some “destroy” method for every 
>>> “create” method.
>>> 
>>> Does it make sense to have the CacheFactory concept at all (especially 
>>> since it is more of a builder)? Could we have some C struct that can be 
>>> used to create the cache, where the struct has fields for all the 
>>> configuration? In general can we rethink the API so that it makes sense for 
>>> C or other language bindings?
>>> 
>>> What about finding a way to be more expressive than void*. Typedefs at 
>>> least express some meaning when hovering but will just decay without 
>>> warning to the void*.
>>> 
>>> Consider:
>>> typedef void* CacheFactory;
>>> typedef void* Cache;
>>> CacheFactory createCacheFactory(void);
>>> void destroyCacheFactory(CacheFactory cacheFactory);
>>> Cache createCache(CacheFactory cacheFactory);
>>> void destroyCache(Cache cache);
>>> 
>>> But this compiles:
>>> CacheFactory cacheFactory = createCacheFactory();
>>> destroyCache(cacheFactory);
>>> 
>>> And explodes at runtime.
>>> Example 
>>> https://github.com/pivotal-jbarrett/geode-native/tree/2d7d0a28fb6f85988e7b921fd96ebb975bad8126/ccache
>>>  
>>> 
>>> 
>>> We could use structs to hold the opaque pointers, and other values.
>>> typedef struct {
>>>   void* ptr;
>>> } CacheFactory;
>>> typedef struct {
>>>   void* ptr;
>>> } Cache;
>>> 
>>> And now this does not compile:
>>> CacheFactory cacheFactory = createCacheFactory();
>>> destroyCache(cacheFactory);
>>> Example 
>>> https://github.com/pivotal-jbarrett/geode-native/tree/23b44e281fd9771474ff3555020710bf23f454ae/ccache
>>>  
>>> 
>>> 
>>> 
>>> -Jake
>>> 
>>> 
 On Mar 24, 2020, at 2:20 PM, Blake Bender >>> > wrote:
 
 Hello everyone,
 
 We'd like to add C bindings to the native client, in preparation for the
 larger task of adding support for new languages, e.g. .net core.  Please
 have a look at the proposal below and let me know what you think.
 
 Thanks,

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

Forgot to include the source example: 
https://github.com/pivotal-jbarrett/geode-native/tree/e47698bad3cf725dcbddaad458813e26e75c8c71/ccache
 



> On Mar 26, 2020, at 2:56 PM, Jacob Barrett  wrote:
> 
> On the strongly typed, I found a solution I like even better.
> 
> struct CacheFactory;
> typedef struct CacheFactory CacheFactory;
> 
> struct Cache;
> typedef struct Cache Cache;
> 
> CacheFactory* createCacheFactory() {
>   return reinterpret_cast(
>   new apache::geode::client::CacheFactory());
> }
> 
> void destroyCacheFactory(CacheFactory* cacheFactory) {
>   delete reinterpret_cast(cacheFactory);
> }
> 
> Cache* createCache(CacheFactory* cacheFactory) {
>   return reinterpret_cast(new apache::geode::client::Cache(
>   reinterpret_cast(cacheFactory)
>   ->create()));
> }
> 
> void destroyCache(Cache* cache) {
>   delete reinterpret_cast(cache);
> }
> 
> The code below is type safe and won’t compile.
> CacheFactory cacheFactory = createCacheFactory();
> destroyCache(cacheFactory);
> 
> 
> -Jake
> 
> 
>> On Mar 26, 2020, at 12:22 PM, Jacob Barrett > > wrote:
>> 
>> I am onboard with the idea but I would like more details in the RFC.
>> 
>> I would prefer that the C bindings be in its own library. The only symbols 
>> that should be visible from this library should be the C exported symbols, 
>> the internal C++ symbols should be hidden.
>> 
>> I feel like this library makes the most sense as a static library. I saw 
>> this because in .NET we would link it as a mixed mode assembly to avoid 
>> having multiple shared libraries to link. If we did a node client I would 
>> expect it too would statically link this library as into its shared library. 
>> Same I imagine would apply to all other language bindings.
>> 
>> How are you planning on managing allocation and deallocation of these opaque 
>> void* pointers. You vaguely showed the allocation but deallocation would be 
>> a good example. I would assume some “destroy” method for every “create” 
>> method.
>> 
>> Does it make sense to have the CacheFactory concept at all (especially since 
>> it is more of a builder)? Could we have some C struct that can be used to 
>> create the cache, where the struct has fields for all the configuration? In 
>> general can we rethink the API so that it makes sense for C or other 
>> language bindings?
>> 
>> What about finding a way to be more expressive than void*. Typedefs at least 
>> express some meaning when hovering but will just decay without warning to 
>> the void*.
>> 
>> Consider:
>> typedef void* CacheFactory;
>> typedef void* Cache;
>> CacheFactory createCacheFactory(void);
>> void destroyCacheFactory(CacheFactory cacheFactory);
>> Cache createCache(CacheFactory cacheFactory);
>> void destroyCache(Cache cache);
>> 
>> But this compiles:
>> CacheFactory cacheFactory = createCacheFactory();
>> destroyCache(cacheFactory);
>> 
>> And explodes at runtime.
>> Example 
>> https://github.com/pivotal-jbarrett/geode-native/tree/2d7d0a28fb6f85988e7b921fd96ebb975bad8126/ccache
>>  
>> 
>> 
>> We could use structs to hold the opaque pointers, and other values.
>> typedef struct {
>>   void* ptr;
>> } CacheFactory;
>> typedef struct {
>>   void* ptr;
>> } Cache;
>> 
>> And now this does not compile:
>> CacheFactory cacheFactory = createCacheFactory();
>> destroyCache(cacheFactory);
>> Example 
>> https://github.com/pivotal-jbarrett/geode-native/tree/23b44e281fd9771474ff3555020710bf23f454ae/ccache
>>  
>> 
>> 
>> 
>> -Jake
>> 
>> 
>>> On Mar 24, 2020, at 2:20 PM, Blake Bender >> > wrote:
>>> 
>>> Hello everyone,
>>> 
>>> We'd like to add C bindings to the native client, in preparation for the
>>> larger task of adding support for new languages, e.g. .net core.  Please
>>> have a look at the proposal below and let me know what you think.
>>> 
>>> Thanks,
>>> 
>>> Blake
>>> 
>>> 
>>> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
>>>  
>>> 
>> 
> 

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

On the strongly typed, I found a solution I like even better.

struct CacheFactory;
typedef struct CacheFactory CacheFactory;

struct Cache;
typedef struct Cache Cache;

CacheFactory* createCacheFactory() {
  return reinterpret_cast(
  new apache::geode::client::CacheFactory());
}

void destroyCacheFactory(CacheFactory* cacheFactory) {
  delete reinterpret_cast(cacheFactory);
}

Cache* createCache(CacheFactory* cacheFactory) {
  return reinterpret_cast(new apache::geode::client::Cache(
  reinterpret_cast(cacheFactory)
  ->create()));
}

void destroyCache(Cache* cache) {
  delete reinterpret_cast(cache);
}

The code below is type safe and won’t compile.
CacheFactory cacheFactory = createCacheFactory();
destroyCache(cacheFactory);


-Jake


> On Mar 26, 2020, at 12:22 PM, Jacob Barrett  wrote:
> 
> I am onboard with the idea but I would like more details in the RFC.
> 
> I would prefer that the C bindings be in its own library. The only symbols 
> that should be visible from this library should be the C exported symbols, 
> the internal C++ symbols should be hidden.
> 
> I feel like this library makes the most sense as a static library. I saw this 
> because in .NET we would link it as a mixed mode assembly to avoid having 
> multiple shared libraries to link. If we did a node client I would expect it 
> too would statically link this library as into its shared library. Same I 
> imagine would apply to all other language bindings.
> 
> How are you planning on managing allocation and deallocation of these opaque 
> void* pointers. You vaguely showed the allocation but deallocation would be a 
> good example. I would assume some “destroy” method for every “create” method.
> 
> Does it make sense to have the CacheFactory concept at all (especially since 
> it is more of a builder)? Could we have some C struct that can be used to 
> create the cache, where the struct has fields for all the configuration? In 
> general can we rethink the API so that it makes sense for C or other language 
> bindings?
> 
> What about finding a way to be more expressive than void*. Typedefs at least 
> express some meaning when hovering but will just decay without warning to the 
> void*.
> 
> Consider:
> typedef void* CacheFactory;
> typedef void* Cache;
> CacheFactory createCacheFactory(void);
> void destroyCacheFactory(CacheFactory cacheFactory);
> Cache createCache(CacheFactory cacheFactory);
> void destroyCache(Cache cache);
> 
> But this compiles:
> CacheFactory cacheFactory = createCacheFactory();
> destroyCache(cacheFactory);
> 
> And explodes at runtime.
> Example 
> https://github.com/pivotal-jbarrett/geode-native/tree/2d7d0a28fb6f85988e7b921fd96ebb975bad8126/ccache
>  
> 
> 
> We could use structs to hold the opaque pointers, and other values.
> typedef struct {
>   void* ptr;
> } CacheFactory;
> typedef struct {
>   void* ptr;
> } Cache;
> 
> And now this does not compile:
> CacheFactory cacheFactory = createCacheFactory();
> destroyCache(cacheFactory);
> Example 
> https://github.com/pivotal-jbarrett/geode-native/tree/23b44e281fd9771474ff3555020710bf23f454ae/ccache
>  
> 
> 
> 
> -Jake
> 
> 
>> On Mar 24, 2020, at 2:20 PM, Blake Bender > > wrote:
>> 
>> Hello everyone,
>> 
>> We'd like to add C bindings to the native client, in preparation for the
>> larger task of adding support for new languages, e.g. .net core.  Please
>> have a look at the proposal below and let me know what you think.
>> 
>> Thanks,
>> 
>> Blake
>> 
>> 
>> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
>>  
>> 
> 

Re: RFC: Add C Bindings to Geode Native Client

[Jacob Barrett]

I am onboard with the idea but I would like more details in the RFC.

I would prefer that the C bindings be in its own library. The only symbols that 
should be visible from this library should be the C exported symbols, the 
internal C++ symbols should be hidden.

I feel like this library makes the most sense as a static library. I saw this 
because in .NET we would link it as a mixed mode assembly to avoid having 
multiple shared libraries to link. If we did a node client I would expect it 
too would statically link this library as into its shared library. Same I 
imagine would apply to all other language bindings.

How are you planning on managing allocation and deallocation of these opaque 
void* pointers. You vaguely showed the allocation but deallocation would be a 
good example. I would assume some “destroy” method for every “create” method.

Does it make sense to have the CacheFactory concept at all (especially since it 
is more of a builder)? Could we have some C struct that can be used to create 
the cache, where the struct has fields for all the configuration? In general 
can we rethink the API so that it makes sense for C or other language bindings?

What about finding a way to be more expressive than void*. Typedefs at least 
express some meaning when hovering but will just decay without warning to the 
void*.

Consider:
typedef void* CacheFactory;
typedef void* Cache;
CacheFactory createCacheFactory(void);
void destroyCacheFactory(CacheFactory cacheFactory);
Cache createCache(CacheFactory cacheFactory);
void destroyCache(Cache cache);

But this compiles:
CacheFactory cacheFactory = createCacheFactory();
destroyCache(cacheFactory);

And explodes at runtime.
Example 
https://github.com/pivotal-jbarrett/geode-native/tree/2d7d0a28fb6f85988e7b921fd96ebb975bad8126/ccache
 


We could use structs to hold the opaque pointers, and other values.
typedef struct {
  void* ptr;
} CacheFactory;
typedef struct {
  void* ptr;
} Cache;

And now this does not compile:
CacheFactory cacheFactory = createCacheFactory();
destroyCache(cacheFactory);
Example 
https://github.com/pivotal-jbarrett/geode-native/tree/23b44e281fd9771474ff3555020710bf23f454ae/ccache
 



-Jake


> On Mar 24, 2020, at 2:20 PM, Blake Bender  wrote:
> 
> Hello everyone,
> 
> We'd like to add C bindings to the native client, in preparation for the
> larger task of adding support for new languages, e.g. .net core.  Please
> have a look at the proposal below and let me know what you think.
> 
> Thanks,
> 
> Blake
> 
> 
> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library

Re: RFC: Add C Bindings to Geode Native Client

[Michael Oleske]

+1

Would certainly be nice since the protobuf work is still mostly
experimental (and then I can continue my goal of Geode-ing all the
languages)

-michael


On Wed, Mar 25, 2020 at 9:03 AM Dan Smith  wrote:

> +1
>
> Great idea! Hey, it's also easy to call into C libraries from Java - maybe
> we can write a java client ;)
>
> It would be nice to see a little bit more detail about the actual API, like
> what does a region put look like?
>
> -Dan
>
> On Wed, Mar 25, 2020 at 8:25 AM Robert Houghton 
> wrote:
>
> > +1
> > YES.
> >
> > Having a set of clean C headers also allows for using a broad set of code
> > generators for additional languages (Swig, for example).
> >
> >
> >
> > On Tue, Mar 24, 2020 at 2:20 PM Blake Bender  wrote:
> >
> > > Hello everyone,
> > >
> > > We'd like to add C bindings to the native client, in preparation for
> the
> > > larger task of adding support for new languages, e.g. .net core.
> Please
> > > have a look at the proposal below and let me know what you think.
> > >
> > > Thanks,
> > >
> > > Blake
> > >
> > >
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
> > >
> >
>

Re: RFC: Add C Bindings to Geode Native Client

[Dan Smith]

+1

Great idea! Hey, it's also easy to call into C libraries from Java - maybe
we can write a java client ;)

It would be nice to see a little bit more detail about the actual API, like
what does a region put look like?

-Dan

On Wed, Mar 25, 2020 at 8:25 AM Robert Houghton 
wrote:

> +1
> YES.
>
> Having a set of clean C headers also allows for using a broad set of code
> generators for additional languages (Swig, for example).
>
>
>
> On Tue, Mar 24, 2020 at 2:20 PM Blake Bender  wrote:
>
> > Hello everyone,
> >
> > We'd like to add C bindings to the native client, in preparation for the
> > larger task of adding support for new languages, e.g. .net core.  Please
> > have a look at the proposal below and let me know what you think.
> >
> > Thanks,
> >
> > Blake
> >
> >
> >
> >
> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
> >
>

Re: RFC: Add C Bindings to Geode Native Client

[Robert Houghton]

+1
YES.

Having a set of clean C headers also allows for using a broad set of code
generators for additional languages (Swig, for example).



On Tue, Mar 24, 2020 at 2:20 PM Blake Bender  wrote:

> Hello everyone,
>
> We'd like to add C bindings to the native client, in preparation for the
> larger task of adding support for new languages, e.g. .net core.  Please
> have a look at the proposal below and let me know what you think.
>
> Thanks,
>
> Blake
>
>
>
> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library
>

RFC: Add C Bindings to Geode Native Client

[Blake Bender]

Hello everyone,

We'd like to add C bindings to the native client, in preparation for the
larger task of adding support for new languages, e.g. .net core.  Please
have a look at the proposal below and let me know what you think.

Thanks,

Blake


https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library