Conversation
_baked.c was the most complicated one. Py3 Unicode implementation requires initialization. I left the Python 2 implementation alone because I'm not entirely sure of the overhead being saved with memcpy right now.
|
No detected performance regression with this. New: Spitfire template 9.65 ms Old: Spitfire template 9.84 ms |
| Py_INCREF(value); | ||
| return value; | ||
| } | ||
| PyObject *args = Py_BuildValue("(N)", value); |
There was a problem hiding this comment.
Can you add a comment on why you are choosing N (and therefore not incrementing the reference count) instead of O?
| static PyTypeObject SanitizedPlaceholderType = { | ||
| PyObject_HEAD_INIT(NULL) | ||
| 0, /* ob_size */ | ||
| PyVarObject_HEAD_INIT(NULL,0 ) |
There was a problem hiding this comment.
| PyVarObject_HEAD_INIT(NULL,0 ) | |
| PyVarObject_HEAD_INIT(NULL, 0) |
| 0, /* tp_members */ | ||
| 0, /* tp_getset */ | ||
| 0, /* tp_base */ | ||
| &PyUnicode_Type, /* tp_base */ |
There was a problem hiding this comment.
Since we're conditionally setting tp_base below, let's not set it here too
| &PyUnicode_Type, /* tp_base */ | |
| 0, /* tp_base */ |
| } | ||
|
|
||
| if (!(PyUnicode_Check(name) || PyString_Check(name))) { | ||
| if (!(PyUnicode_Check(name) || PyBytes_Check(name))) { |
There was a problem hiding this comment.
I think you need to either include bytesobject.h for python2.6+ compatibility of PyBytes_Check or add a PY_MAJOR_VERSION check here like you do elsewhere
| } | ||
|
|
||
| if (!(PyUnicode_Check(name) || PyString_Check(name))) { | ||
| if (!(PyUnicode_Check(name) || PyBytes_Check(name))) { |
There was a problem hiding this comment.
I think you need to either include bytesobject.h for python2.6+ compatibility of PyBytes_Check or add a PY_MAJOR_VERSION check here like you do elsewhere
nicksay
left a comment
nicksay
left a comment
There was a problem hiding this comment.
Just a couple of changes, I think
Co-Authored-By: Alex Nicksay <nicksay@google.com>
3 participants
_baked.c was the most complicated one. Py3 Unicode implementation
requires initialization. I left the Python 2 implementation alone
because I'm not entirely sure of the overhead being saved with memcpy
right now.