karllolee / py-leveldb

Automatically exported from code.google.com/p/py-leveldb
Other
0 stars 0 forks source link

Return None from LevelDB.Get rather than raising KeyError (performance) #19

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This would break API compatibility, though in a relatively minor way:

My application issues a lot of Gets for keys that don't exist, and handling the 
KeyError takes more time than checking for None.

A simple test (one million inserts; one million Gets of a missing key) has 
exception handling taking 65% longer, 5.22s vs 3.39s.

The only change I made was the simple patch:
--- leveldb_object.cc   (revision 48)
+++ leveldb_object.cc   (working copy)
@@ -271,8 +271,8 @@
    PY_LEVELDB_RELEASE_BUFFER(key);

    if (status.IsNotFound()) {
-       PyErr_SetNone(PyExc_KeyError);
-       return 0;
+       Py_INCREF(Py_None);
+       return Py_None;
    }

    if (!status.ok()) {

Original issue reported on code.google.com by pe...@teichman.org on 26 Jun 2012 at 2:33

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Cool numbers.

Breaking compatability now is not an option now, I think.

Perhaps a new function (Get2())? Or a flag?

Original comment by arnim...@gmail.com on 26 Jun 2012 at 10:30

GoogleCodeExporter commented 9 years ago
That API is so compact and clean, I think Get2() would probably stick out.

Maybe the most flexible option is to match Python's dict get() and take a 
default value in kwargs:

db.Get("missing_key", default=None)

Original comment by pe...@teichman.org on 26 Jun 2012 at 11:33

GoogleCodeExporter commented 9 years ago
I like the second one. Will do.

Original comment by arnim...@gmail.com on 26 Jun 2012 at 11:51

GoogleCodeExporter commented 9 years ago

Original comment by arnim...@gmail.com on 26 Jun 2012 at 2:28