Issue 8124 - Patch "Context for compare functions"
Summary: Patch "Context for compare functions"
Status: VERIFIED DUPLICATE of issue 7980
Alias: None
Product: LMDB
Classification: Unclassified
Component: liblmdb (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-02 21:46 UTC by rouzier@gmail.com
Modified: 2021-06-02 15:04 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description rouzier@gmail.com 2015-05-02 21:46:00 UTC
Full_Name: James Rouzier
Version: LMDB mdb.master
OS: N/A
URL: https://github.com/rouzier/openldap/commit/33f1dc8be4a8503e78930f130ff9c8e5aa9bfa7b
Submission from: (NULL) (70.81.32.80)


New feature inclusion request: "Context for compare functions"

I am finishing up a new perl module which is a very thin wrapper for LMDB.
(The current perl LMDB module does not fit my needs as it does not support
NO_TLS really support).

When looking at mdb_set_compare and mdb_set_dupsort.

There were three choices.

* Not support mdb_set_compare, mdb_set_dupsort in the module.
* Capture each mdb_*(del|put|get) function to figure which dbi I am using.
* Add support for contexts in the compare functions.

So I decided on the latter.

I understand that passing a context for each compare can add some overhead.
So I made this feature optional it is only enabled if the macro MDB_CMP_CTX is
enabled.

This patch does the following.

* Adds context to the MDB_cmp_func

   typedef int  (MDB_cmp_func)(const MDB_val *a, const MDB_val *b ,void* ctx);

* And two new data members to the MDB_dbx struct
  * void        *md_cmpctx;     /**< user-provided context for md_cmp */
  * void        *md_dcmpctx;    /**< user-provided context for md_dcmp */

* Add functions for setting/getting the contexts 

  * int  mdb_set_cmpctx(MDB_txn *txn, MDB_dbi dbi, void *ctx);

  * int  mdb_set_dcmpctx(MDB_txn *txn, MDB_dbi dbi, void *ctx);

  * int  mdb_get_cmpctx(MDB_txn *txn, MDB_dbi dbi, void **ctx);

  * int  mdb_get_dcmpctx(MDB_txn *txn, MDB_dbi dbi, void **ctx);


I have no problem maintaining this as a separate patch if it does not fit in the
vision of LMDB.
I believe this can be useful for people who wants to add LMDB to a scripting
language.

New feature inclusion request: "Context for compare functions"

I am finishing up a new perl module which is a very thin wrapper for LMDB.
(The current perl LMDB module does not fit my needs as it does not support
NO_TLS really well).

When looking at mdb_set_compare and mdb_set_dupsort.

There were three choices.

* Not support mdb_set_compare, mdb_set_dupsort in the module.
* Capture each mdb_*(del|put|get) function to figure which dbi I am using.
* Add support for contexts in the compare functions.

So I decided on the latter.

I understand that passing a context for each compare can add some overhead.
So I made this feature optional it is only enabled if the macro MDB_CMP_CTX is
enabled.

This patch does the following.

* Adds context to the MDB_cmp_func

   typedef int  (MDB_cmp_func)(const MDB_val *a, const MDB_val *b ,void* ctx);

* And two new data members to the MDB_dbx struct
  * void        *md_cmpctx;     /**< user-provided context for md_cmp */
  * void        *md_dcmpctx;    /**< user-provided context for md_dcmp */

* Add functions for setting/getting the contexts 

  * int  mdb_set_cmpctx(MDB_txn *txn, MDB_dbi dbi, void *ctx);

  * int  mdb_set_dcmpctx(MDB_txn *txn, MDdbi i dbi, void *ctx);

  * int  mdb_get_cmpctx(MDB_txn *txn, MDB_dbi dbi, void **ctx);

  * int  mdb_get_dcmpctx(MDB_txn *txn, MDB_dbi dbi, void **ctx);


I have no problem maintaining this as a separate patch if it does not fit in the
vision of LMDB.
I figured it might be very useful for others.

Thank You for creating and releasing LMDB

I, James Rouzier, hereby place the following modifications to OpenLDAP Software
(and only these modifications) into the public domain. Hence, these
modifications may be freely used and/or redistributed for any purpose with or
without attribution and/or other notice.
Comment 1 Howard Chu 2015-05-02 22:04:09 UTC
rouzier@gmail.com wrote:
> Full_Name: James Rouzier
> Version: LMDB mdb.master
> OS: N/A
> URL: https://github.com/rouzier/openldap/commit/33f1dc8be4a8503e78930f130ff9c8e5aa9bfa7b
> Submission from: (NULL) (70.81.32.80)
>
>
> New feature inclusion request: "Context for compare functions"
>
> I am finishing up a new perl module which is a very thin wrapper for LMDB.
> (The current perl LMDB module does not fit my needs as it does not support
> NO_TLS really support).
>
> When looking at mdb_set_compare and mdb_set_dupsort.
>
> There were three choices.
>
> * Not support mdb_set_compare, mdb_set_dupsort in the module.
> * Capture each mdb_*(del|put|get) function to figure which dbi I am using.
> * Add support for contexts in the compare functions.
>
> So I decided on the latter.
>
> I understand that passing a context for each compare can add some overhead.
> So I made this feature optional it is only enabled if the macro MDB_CMP_CTX is
> enabled.

I don't see supporting compare functions in interpreted languages being 
a realistic/worthwhile thing to do. The difference in performance is 
orders of magnitude.

I also don't believe such conditional compilation is a good idea. LMDB 
is designed to be simple, to have very few options, and to Just Work the 
same on every platform. Conditionally compiled features are horrible 
because you can't guess which features will be present in a 
system-installed library.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 2 rouzier@gmail.com 2015-05-02 23:36:39 UTC
Howard thank you for taking the time to explain your reasoning.

I am curious if I were remove the conditional compilation would the patch
be more palatable.
Because usually if you are using an interpreted language you are already
taking a performance hit.
Also it only slows your program down if you use the compare functions.

If answer is still no I will just maintain the patch in my own branch.

James Rouzier.






On Sat, May 2, 2015 at 6:04 PM, Howard Chu <hyc@symas.com> wrote:

> rouzier@gmail.com wrote:
>
>> Full_Name: James Rouzier
>> Version: LMDB mdb.master
>> OS: N/A
>> URL:
>> https://github.com/rouzier/openldap/commit/33f1dc8be4a8503e78930f130ff9c8e5aa9bfa7b
>> Submission from: (NULL) (70.81.32.80)
>>
>>
>> New feature inclusion request: "Context for compare functions"
>>
>> I am finishing up a new perl module which is a very thin wrapper for LMDB.
>> (The current perl LMDB module does not fit my needs as it does not support
>> NO_TLS really support).
>>
>> When looking at mdb_set_compare and mdb_set_dupsort.
>>
>> There were three choices.
>>
>> * Not support mdb_set_compare, mdb_set_dupsort in the module.
>> * Capture each mdb_*(del|put|get) function to figure which dbi I am using.
>> * Add support for contexts in the compare functions.
>>
>> So I decided on the latter.
>>
>> I understand that passing a context for each compare can add some
>> overhead.
>> So I made this feature optional it is only enabled if the macro
>> MDB_CMP_CTX is
>> enabled.
>>
>
> I don't see supporting compare functions in interpreted languages being a
> realistic/worthwhile thing to do. The difference in performance is orders
> of magnitude.
>
> I also don't believe such conditional compilation is a good idea. LMDB is
> designed to be simple, to have very few options, and to Just Work the same
> on every platform. Conditionally compiled features are horrible because you
> can't guess which features will be present in a system-installed library.
>
> --
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP  http://www.openldap.org/project/
>
Comment 3 Howard Chu 2021-06-02 13:20:41 UTC

*** This issue has been marked as a duplicate of issue 7980 ***