danburkert / lmdb-rs

Safe Rust bindings for LMDB
Apache License 2.0
170 stars 87 forks source link

Consider merging with lmdb_rs #4

Open iqualfragile opened 8 years ago

iqualfragile commented 8 years ago

https://github.com/vhbit/lmdb-rs

tarcieri commented 8 years ago

I don't really understand why there are two LMDB crates, but I mostly prefer this one's API so far (I've tried them both)

vks commented 7 years ago

What are the differences?

tarcieri commented 7 years ago

And now there's this! https://github.com/fullcontact/lmdb-zero

vks commented 7 years ago

At least it gives a rationale:

The main issue with the existing crates is that they try to abstract some properties of LMDB away, and as a result are not able to expose some of LMDB's functionality, and in some cases compromise safety.

lmdb-zero is instead as much as possible a 1:1 mapping of the raw API, mainly providing RAII constructs and integration into Rust's borrow checker to ensure safety.

tarcieri commented 7 years ago

Would love to hear specific cases where this crate is doing that /cc @fullcontact

iqualfragile commented 7 years ago

review on the sibling issue: https://github.com/vhbit/lmdb-rs/issues/32#issuecomment-310906601

cmbrandenburg commented 7 years ago

I wrote a summary of the differences between this crate and https://github.com/vhbit/lmdb-rs.

My conclusion is that the vhbit crate is a dead-end or else requires breaking changes to its API that will effectively make it redundant with this crate.

Follow the link for details, though I summarize the differences here:

  1. The vhbit crate is memory-unsafe. This is a flat-out no-go.
  2. The vhbit crate has a more complex database-handle abstraction that provides no benefits and incurs additional cost.
  3. The vhbit crate does not allow applications to generically handle read-only vs read-write cursors and transactions.
  4. The vhbit crate, overall, has some aesthetic API design choices that I dislike compared to this crate's.

In my opinion, the vhbit crate provides no benefits over what this crate provides and it has many disadvantages with no clear path to distinguishing itself as an advantageous crate. I see no reason for this crate to merge with it.

As for lmdb-zero, I haven't made a deep analysis but am intrigued with the idea of an LMDB crate providing access to all the low-level nuts and bolts of the C API. Superficially, I like the idea of a single crate that merges the low-level access with higher-level abstractions. Specifically, however, I don't know what that would entail or even if it makes sense.

danburkert commented 7 years ago

Thanks for the thorough writeup, @cmbrandenburg! You're findings mirror my own, and those of people I've talked too, but it's great to have it thoroughly researched and summarized. I've resisted commenting on this issue up till now because I feel like the crates can stand on their own, but it certainly helps new comers when evaluating to have impartial sources spend effort and share their findings. Thanks again, and thanks for taking the effort to send issues / PRs to both crates.

cmbrandenburg commented 7 years ago

@danburkert, you're welcome.

I don't see myself doing a deep analysis on lmdb-zero anytime soon—though if I were starting over on my LMDB application, I would for sure take a look at it.

What are your thoughts on merging with that crate? I very much like the idea of a crate that provides both a high-level and low-level abstraction on top of LMDB.

danburkert commented 7 years ago

What are your thoughts on merging with that crate? I very much like the idea of a crate that provides both a high-level and low-level abstraction on top of LMDB.

Are you asking about lmdb-zero in particular?

I'm not keen on merging this crate with another. lmdb is pretty much in maintenance-mode; all major features have been implemented, and it works well. There are of course outstanding issues, but overall the crate isn't seeing too much churn. I'd consider releasing a 1.0 version, except that I don't think that's wise when lmdb itself is < 1.0. In this context I don't see an upside to merging with another crate. I think it's healthy for the crates ecosystem to have multiple choices for solving the same problem, I don't buy into the need to unify around one crate, especially for something that can realistically be called 'done', like an lmdb wrapper.

As far as the things lmdb-zero claims about 'other lmdb crates' on their README: I don't think they apply to this crate. lmdb-zero provides zero concrete examples, however, so it's difficult to say. Some of the things contained in that README are interesting, though:

The main issue with the existing crates is that they try to abstract some properties of LMDB away, and as a result are not able to expose some of LMDB's functionality

The goal of lmdb is to abstract away the unsafety of LMDB. It does this purely through zero-cost abstractions. Despite this, it is possible to drop down to raw LMDB using the lmdb-sys API. Every higher-level abstraction that lmdb provides also allows access to the raw underlying LMDB (lmdb-sys) type:

Database Environment Transaction Cursor

This should have been obvious to anyone that read through the lmdb API, so I'm inclined to believe the authors of lmdb-zero have not.

and in some cases compromise safety.

lmdb has no-known memory safety issues, it would be considered a high priority bug if one were found. There are unsafe parts of the API, but they are marked as unsafe, and they only exist because I haven't found a way around the unsafety. That constitutes a small part of the overall crate's API surface, though.

One thing to remember is that, when it comes to open source projects, inactivity != abandoned and activity != quality

cmbrandenburg commented 7 years ago

@danburkert, thanks.

devinrsmith commented 5 years ago

Throwing this in the mix https://github.com/mozilla/rkv . Not that it necessarily relates to the original issue of merging with other libraries, but might be relevant as a point of comparison/learnings.

Edit: looks like rkv depends on their own fork of this repo: https://github.com/mozilla/lmdb-rs