Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async branch getting close #9

Open
carter-thaxton opened this issue Jun 14, 2011 · 1 comment
Open

Async branch getting close #9

carter-thaxton opened this issue Jun 14, 2011 · 1 comment

Comments

@carter-thaxton
Copy link
Contributor

Hey all,

I've completely implemented node-leveldb as async.
It currently supports Open, Close, Put, Del, Get, and Write (batch).
That's all that was implemented as sync anyway. Rather than get ahead of myself, I figured now's the time to sync up.

Check out this branch:
http://github.com/carter-thaxton/node-leveldb/tree/async

It still needs some work, but I think I'll be consolidating this into a large-ish pull request soon, to get this going on the main project branch.

At one point, Samori (shinuza) pinged me and said that you were considering having a JS layer above the C++ module layer. I think that's a good idea. He mentioned, however, that you were considering using the leveldb argument orderings instead of the more node-friendly orderings, e.g. path before options, in the bindings and then swapping them in the JS layer. I went ahead and made the interface of the C++ bindings match up with the proposed interface in leveldb.js. This includes accepting optional options objects in the expected positions. It wasn't too tough, and it leaves room for various optimizations if we do this in the C++ code.

Randall, I think you were excited about finding a way to more transparently pass strings and objects down from the v8 thread to the EIO functions. I'm all ears. One thing I'm worried about is that v8 doesn't like you to use objects from another thread, even if it's just for read. Scavenging and other operations end up moving the memory referred to by handles. So, that means we need to copy the strings and other values out of the Arguments, and into some temporary location for the lifetime of the function. I've done that with a hierarchy of Params objects. You'll see what I mean in the code.

Feel free to poke holes...

Thanks,
Carter

@creationix
Copy link
Owner

I'm not qualified to review this code, but I'd really like to pull it in. Can someone else review?

I'm particularly interested to see if leveldb is really thread safe as advertised and we can allow concurrent access using this. Since the operations are using node's thread pool, it should just work unless there is some special leveldb c++ api to manually sync the threads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants