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

Replace leveldb with lmdb #386

Closed
wants to merge 2 commits into from
Closed

Replace leveldb with lmdb #386

wants to merge 2 commits into from

Conversation

mavenlin
Copy link
Contributor

@mavenlin mavenlin commented May 4, 2014

One quite annoying thing of caffe is that the leveldb can only be accessed by one process at a time. For big database like imagenet, we need to duplicate the leveldb to run multiple models on different cards on one machine. lmdb is faster than leveldb [http://symas.com/mdb/microbench/], and most importantly, it supports multiprocess concurrency [http://symas.com/mdb/#features].

It is released under OpenLDAP license, which is a BSD-style license.

@niuzhiheng
Copy link
Contributor

@mavenlin Cool!

@sguada
Copy link
Contributor

sguada commented May 4, 2014

@mavenlin nice idea, but could you create a new data layer that read from lmdb(in the future it will be a different data source as #238).
A different layer would allow to keep leveldb back compatibility and benchmark side by side for our cases.

@shelhamer
Copy link
Member

One quite annoying thing of caffe is that the leveldb can only be accessed by one process at a time.

Agreed. But @sguada makes a good point, and this should be an addition and not a replacement, at least for the beginning. (Sergio: I've assigned you as the reviewer for this PR.)

@jeffdonahue
Copy link
Contributor

Also agreed that it's annoying that a leveldb can't have more than one reader at a time. As a hacky workaround, feel free to use this bash script to "clone" the leveldb using softlinks to the sstable data files (so hardly any extra disk is actually used for the clone). It takes 2 args: one an existing leveldb dir path and one a new path. This workaround is still kind of annoying to use as you still have to edit your prototxts to use the cloned leveldb dir.

I'd never heard of lmdb but assuming its performance really is as good or better than leveldb and it doesn't have any other issues that leveldb eases, I'd definitely be in favor of supporting it as well as leveldb.

@kloudkl
Copy link
Contributor

kloudkl commented May 5, 2014

Quoting the Wikipedia article, the open/closed principle states "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification".

@Yangqing
Copy link
Member

Yangqing commented May 7, 2014

Thumbs up for adding more databases & thumbs up for keeping leveldb still
in the game. Leveldb is quite fast and is tested by multiple parties,
making it good to keep.

On a separate note, we may want to do some discussion and decide a
"preferred database" that we use in default - for example I know Sergey
likes MongoDB.

Yangqing

On Mon, May 5, 2014 at 4:32 AM, kloudkl notifications@github.com wrote:

Quoting the Wikipedia articlehttp://en.wikipedia.org/wiki/Open/closed_principle,
the open/closed principlehttps://www.google.co.in/#newwindow=1&q=open+to+extension+closed+to+modificationstates "software entities (classes, modules, functions, etc.) should be
open for extension, but closed for modification".

Reply to this email directly or view it on GitHubhttps://github.com//pull/386#issuecomment-42178948
.

@sergeyk
Copy link
Contributor

sergeyk commented May 21, 2014

@mavenlin If you'll rebase on latest dev, and contribute this as not a replacement but an addition to leveldb, we will gladly merge! Simplest would be adding a flag to data_layer, and forgetting about the tools/ change -- let that use leveldb.

@mavenlin mavenlin closed this May 21, 2014
@mavenlin mavenlin deleted the lmdb branch May 21, 2014 16:26
@mavenlin mavenlin mentioned this pull request May 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants