Skip to content

Commit

Permalink
bad C API
Browse files Browse the repository at this point in the history
  • Loading branch information
vstinner committed Jul 30, 2018
1 parent 6d8ea97 commit 2e2a3cf
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 17 deletions.
47 changes: 30 additions & 17 deletions doc/bad_api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ See also :ref:`Remove functions <remove-funcs>`.
Borrowed references
===================

CPython 3.7 has 36 functions and macros which return borrowed references:
CPython 3.7 has 36 functions and macros which return borrowed references.

CPython contains ``Doc/data/refcounts.dat`` (file is edited manually) which
documents how functions handle reference count.

Functions
---------

* ``PyCell_GET()``
* ``PyDict_GetItem()``
* ``PyDict_GetItemWithError()``
* ``PyDict_GetItemString()``
Expand All @@ -34,47 +39,50 @@ CPython 3.7 has 36 functions and macros which return borrowed references:
* ``Py_InitModule3()``
* ``Py_InitModule4()``
* ``PyImport_GetModuleDict()``
* ``PyList_GET_ITEM()``
* ``PyList_GetItem()``
* ``PyMethod_Class()``
* ``PyMethod_Function()``
* ``PyMethod_GET_CLASS()``
* ``PyMethod_GET_FUNCTION()``
* ``PyMethod_GET_SELF()``
* ``PyMethod_Self()``
* ``PyModule_GetDict()``
* ``PyNumber_Check()``
* ``PyObject_Init()``
* ``PySequence_Fast_GET_ITEM()``
* ``PySys_GetObject()``
* ``PySys_GetXOptions()``
* ``PyThreadState_GetDict()``
* ``PyTuple_GET_ITEM()``
* ``PyTuple_GetItem()``
* ``PyWeakref_GET_OBJECT()``
* ``PyWeakref_GetObject()``

CPython contains ``Doc/data/refcounts.dat`` (file is edited manually) which
documents how functions handle reference count.
Macros
------

* ``PyCell_GET()``: access directly ``PyCellObject.ob_ref``
* ``PyList_GET_ITEM()``: access directly ``PyListObject.ob_item``
* ``PyMethod_GET_CLASS()``
* ``PyMethod_GET_FUNCTION()``: access directly ``PyMethodObject.im_func``
* ``PyMethod_GET_SELF()``: access directly ``PyMethodObject.im_self``
* ``PySequence_Fast_GET_ITEM()``: use ``PyList_GET_ITEM()``
or ``PyTuple_GET_ITEM()``
* ``PyTuple_GET_ITEM()``: access directly ``PyTupleObject.ob_item``
* ``PyWeakref_GET_OBJECT()``: access directly ``PyWeakReference.wr_object``

``PyObject**``
==============

Array of pointers to Python objects (``PyObject**``)
====================================================

``PyObject**`` must not be exposed: ``PyObject** PySequence_Fast_ITEMS(ob)``
has to go.

Evil PyDict_GetItem()
=====================
PyDict_GetItem()
================

The ``PyDict_GetItem()`` API is one of the most commonly called function but
it has multiple flaws:

* it returns a :ref:`borrowed reference <borrowed-ref>`
* it ignores any kind of error: it calls ``PyErr_Clear()``

The lookup is surrounded by ``PyErr_Fetch()`` and ``PyErr_Restore()`` to ignore
any exception.
The dictionary lookup is surrounded by ``PyErr_Fetch()`` and
``PyErr_Restore()`` to ignore any exception.

If hash(key) raises an exception, it clears the exception and just returns
``NULL``.
Expand Down Expand Up @@ -120,12 +128,17 @@ See also ``PyLong_AsLongAndOverflow()``.
Open questions
==============

.. _refcount:

Reference counting
------------------

Should we do something for reference counting, Py_INCREF and Py_DECREF?
Replace them with function calls at least?

See :ref:`Change the garbage collector <change-gc>` and :ref:`Py_INCREF
<incref>`.

``PyObject_CallFunction("O")``
------------------------------

Expand Down
3 changes: 3 additions & 0 deletions doc/new_api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ Py_INCREF()
The open question remains if it will be possible to replace ``Py_INCREF()`` and
``Py_DECREF()`` with function calls without killing performances.

See :ref:`Reference counting <refcount>` and :ref:`Change the garbage collector
<change-gc>`.

Hide C structures
-----------------

Expand Down
4 changes: 4 additions & 0 deletions doc/optimization_ideas.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Remove debug checks
See :ref:`Regular runtime <regular-runtime>`: since a debug runtime will be
provided by default, many sanity checks can be removed from the release build.

.. _change-gc:

Change the garbage collector: unlikely
======================================

Expand All @@ -30,6 +32,8 @@ the C API, which is out of the scope of the :ref:`new C API project
<new-c-api>`. The main risk is to break too many C extensions which would make
this idea unsuable in practice.

See also :ref:`Reference counting <refcount>`.

Remove the GIL: unlikely
========================

Expand Down

0 comments on commit 2e2a3cf

Please sign in to comment.