search

jimbraun / XCDF

XCDF: eXplicitly Compacted Data Format. See documentation at Read the Docs:
https://xcdf.readthedocs.io/en/latest/
Other
14 stars 8 forks source link

pybinding memory leaks and XCDFFieldsByNameSelector slowness #37

Closed jimbraun closed 9 years ago

jimbraun commented 9 years ago

A quick examination of the code reveals mostly proper INCREF and DECREF, but a closer inspection is required. For example, with PyArg_ParseTuple(args, format, &filename, &filemode), are we given these references or are they just loaned? Note that XCDFFieldsByNameSelector and XCDFTupleSetter appear as though they leak, as they have a PyObject as a member, but actually do not because the reference is subsequently given away, not copied, in GetTuple()

XCDFFieldsByNameSelector parses the field specification string for each entry. This is slow, much slower than just reading all the fields. This behavior must be improved. The best option is probably to put an XCDFFieldsByNameSelector* in the XCDFFieldIterator object. We can't create a new PyTuple in operator() (since this is called many times per event), so create a new PyTuple on construction and when GetTuple() is called. Possibly wrap this in another object. XCDFFieldsByNameSelector would need a destructor and have copy/assignment disallowed.

jimbraun commented 9 years ago

OK, memory check complete.

PyArg_ParseTuple returns borrowed references that don't need DECREF. PyTuple_SetItem takes over ownership of the references. Check references again after fixing the XCDFFieldsByNameSelector issue.

jimbraun commented 9 years ago

Fixed compiler warnings as well. Need -no-strict-aliasing when compiling the pybindings because python breaks strict-aliasing rules.