[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: Hi, Thanks for finding those issues. I attached a new patch. a) Good find, I added the free() for gdbm. ndbm doesn't need free(). b) Added the error check. I don't know if a test can be made for this. If there was a common way to patch C libraries in CPython, I would give that a try. Also, do you know if we should check for the gdbm error before the dbm error, or does it not matter? c) Yes, it looks like NULL is used here instead of 0. Changed 0s to NULLS. Let me know if you see anything else. -- Added file: http://bugs.python.org/file35150/dbm_bool_e.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: Also, I'm happy to allow the code to be ported to pybsddb. As long as it doesn't cause any problems with CPython licensing - and I can't think of any way it could. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: I did try the suggestion to return Py_False, but that gives the wrong result since Py_False is not 0 and gets returned as Py_True. I looked for similar code, and this looks like the convention for handling if obj. PyObject_IsTrue() is called on the object. What it returns can be affected by an nb_bool function that returns 0 or 1 (or -1). Here are a few similar examples that need to implement nb_bool to handle converting an obj to a bool: Objects/floatobject.c:float_bool Modules/_datetimemodule.c:delta_bool Objects/complexobject.c:complex_bool For many types, there is no nb_bool, and other things are tried such as getting the length. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: Thank you for the feedback. Sorry I didn't see your previous response until today. I will take a look and respond tonight. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: Hi Jesús, I believe the patch should have this behavior: a) If the database is closed, raise an exception. b) If database is empty, return False. c) If database has any entry, return True. Fast and simply checking if the database has at least a single record. The 0/1 appears to be converted to Py_TRUE/Py_FALSE in PyObject_IsTrue() in boolobject.c. For background, here's the code path I'm seeing: bool_new(...)(in Objects/boolobject.c) ... ok = PyObject_IsTrue(obj) (in Objects/object.c) ... return PyBool_FromLong(ok) Looking at PyObject_IsTrue is informative. It shows that since tp_as_number-nb_bool is defined (as dbm_bool), it is used instead of the old default of tp_as_mapping-mp_length. I'm still learning CPython, and I'm not sure if everything goes through bool_new(), but I think this makes sense. If you see more places I can/should look for details here, let me know. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: New patch with Pep 7 fix - no c++ // style comments. -Thanks johansen. -- Added file: http://bugs.python.org/file34916/dbm_bool_d.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: The performance is still an issue in python 3. Attaching a patch for python 3, performance numbers are below. Measuring ndbm time for a falsey check on an open db with 100 entries. gdbm performance is similar. Before patch: db is not None: 6.9141387939453125e-06 not db: 0.0006985664367675781 Factor: 101X (slow) After patch: db is not None: 4.76837158203125e-06 not db: 4.0531158447265625e-06 Factor: 1X (expected) Also, added a couple tests to verify bool(db) returns the correct value. -- nosy: +eolson Added file: http://bugs.python.org/file34879/dbm_bool.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: Uploading patch with minor test changes (dbm_bool_b.patch). Backwards compatibility note: Result of running bool(db) on a db that has been closed: Old: _dbm.error: DBM object has already been closed With patch: returns False instead of raising. I think this is desireable, but we could have bool(db) raise an exception when the db is closed if that is preferred for backwards compatibility. -- Added file: http://bugs.python.org/file34886/dbm_bool_b.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2159] dbmmodule inquiry function is performance prohibitive
Eric Olson added the comment: Make the changes backward compatible after getting input on possible problems from r.david.murray patch: dbm_bool_c.patch Now, the only change should be faster performance for bool(db). -- versions: +Python 3.5 Added file: http://bugs.python.org/file34895/dbm_bool_c.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2159 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
