[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Binding mdb_set_compare with Go



Actually I’m not commenting on binding Go but I’m voting for a context passed to the compare function.

I fully agree that the compare function is part of the critical path. But as I need to define custom indexes with compound keys the compare functions varies and it would be impractical to predefine for any compound key combination a c function. 

The compare context would be stored on the struct MDB_dbx.

typedef struct MDB_dbx {
	MDB_val		md_name;		/**< name of the database */
	MDB_cmp_func	*md_cmp;	/**< function for comparing keys */
	void            *md_cmpctx; /** user-provided context for md_cmp **/
	MDB_cmp_func	*md_dcmp;	/**< function for comparing data items */
	void            *md_dcmpctx;/** user-provided context for md_dcmp **/
	MDB_rel_func	*md_rel;	/**< user relocate function */
	void		*md_relctx;		/**< user-provided context for md_rel */
} MDB_dbx;

The following is a draft (not tested yet) of a generic compare function. The context contains a compare specification which is a null terminated list of <type><order> pairs.

// compareSpec <type><order>...<null>
int key_comp_generic(const MDB_val *a, const MDB_val *b, char *compareSpec) {
    int result = 0;
    char *pa = a->mv_data;
    char *pb = b->mv_data;

    while (1) {
        switch (*compareSpec++) {
           case 0:
                break;
           case INT32_KEY :
           {
                unsigned int va = *(unsigned int *)pa;
                unsigned int vb = *(unsigned int *)pb;
                
                if (*compareSpec++ == ASCENDING_ORDER) {
                    result = (va < vb) ? -1 : va > vb;
                }
                else {
                    result = (va > vb) ? -1 : va < vb;
                }
                
                if (result != 0) {
                    break;
                }
                else {
                    pa += 4;
                    pb += 4;
                }
            }
            case INT64_KEY :
            {
                unsigned long long va = *(unsigned long long *)pa;
                unsigned long long vb = *(unsigned long long *)pb;

                if (*compareSpec++ == ASCENDING_ORDER) {
                    result = (va < vb) ? -1 : va > vb;
                }
                else {
                    result = (va > vb) ? -1 : va < vb;
                }
                
                if (result != 0) {
                    break;
                }
                else {
                    pa += 8;
                    pb += 8;
                }
            }
            case STRING_KEY :
            {
                unsigned int la = *(unsigned int *)pa;
                unsigned int lb = *(unsigned int *)pb;
                
                pa += 4;
                pb += 4;

                if (*compareSpec++ == ASCENDING_ORDER) {
                    result = strncmp(pa, pb, (la < lb) ? la : lb);
                    if (result != 0) {
                        break;
                    }
                    else {
                        result = (la < lb) ? -1 : la > lb;
                    }
                }
                else {
                    result = strncmp(pb, pa, (la < lb) ? la : lb);
                    if (result != 0) {
                        break;
                    }
                    else {
                        result = (la > lb) ? -1 : la < lb;
                    }
                }
                
                if (result != 0) {
                    break;
                }
                else {
                    pa += la;
                    pb += lb;
                }
            }
        }
    }
    
    return result;
}

Regards
Juerg


On 29/10/15 10:40, "openldap-technical on behalf of Howard Chu" <openldap-technical-bounces@openldap.org on behalf of hyc@symas.com> wrote:

>Bryan Matsuo wrote:
>> openldap-technical,
>>
>> I am working on some Go (golang) bindings[1] for the LMDB library and I have
>> some interest in exposing the functionality of mdb_set_compare (and
>> mdb_set_dupsort). But it is proving difficult and I have a question about the
>> function(s).
>>
>> Calling mdb_set_compare from the Go runtime is challenging. Using C APIs with
>> callbacks comes with restrictions[2][3]. I believe it impossible to bind these
>> functions way that is flexible, as one would expect. A potential change to
>> LMDB that would make binding drastically easier is having MDB_cmp_func to take
>> a third "context" argument with type void*. Then a binding could safely use an
>> arbitrary Go function for comparisons.
>>
>> Is it possible for future versions of LMDB to add a third argument to the
>> MDB_cmp_func signature? Otherwise would it be acceptable for a variant API to
>> be added using a different function type, one accepting three arguments?
>>
>> Thanks for the consideration.
>>
>> Cheers,
>> - Bryan
>>
>> [1] Go bindings -- https://github.com/bmatsuo/lmdb-go
>> [2] Cgo pointer restrictions --
>> https://github.com/golang/proposal/blob/master/design/12416-cgo-pointers.md
>> [3] Cgo documentation -- https://golang.org/cmd/cgo/
>
>I see nothing in these restrictions that requires extra information to be 
>passed from Go to C or from C to Go.
>
>There is a vague mention in [2]
>
>"A particular unsafe area is C code that wants to hold on to Go func and 
>pointer values for future callbacks from C to Go. This works today but is not 
>permitted by the invariant. It is hard to detect. One safe approach is: Go 
>code that wants to preserve funcs/pointers stores them into a map indexed by 
>an int. Go code calls the C code, passing the int, which the C code may store 
>freely. When the C code wants to call into Go, it passes the int to a Go 
>function that looks in the map and makes the call."
>
>But it's nonsense in this case - you want to pass a Go function pointer to C, 
>but the only way for C to use it is to call some *other* Go function? Sorry 
>but there is no other Go function for the mdb_cmp() function to call, the only 
>one it knows about is the function pointer that you pass.
>
>If this is what you're referring to, adding a context pointer doesn't achieve 
>anything. If this isn't what you're referring to, then please explain exactly 
>what you hope to achieve with this context pointer.
>
>-- 
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP  http://www.openldap.org/project/