perf: load results by row rather than by column

In the past we were converting results to Python tuples proceeding
column by column. The rationale was that certain overhead such as
selecting the loader was to be paid only once per column, not once per
datum.

However this analysis was dismantled in #1163, see especially the comment at
https://github.com/psycopg/psycopg/pull/1163#issuecomment-3288921422
for some benchmark comparing various conversion strategies. In the end,
the simple row-by-row loading in a single function ends up being more
performing. Performance now surpasses the one of psycopg2.

See also #1155 for an initial analysis of performance regression.

A big thank you to Jörg Breitbart (@jerch) for this improvement!

diff --git a/docs/news.rst b/docs/news.rst
index 5bca48cb9bbc16ec797f12d0511716aaac24966a..74450d4c846d9bf7f19c68f88c5cf312eb1bc97b 100644 (file)
--- a/docs/news.rst
+++ b/docs/news.rst
@@ -53,6 +53,7 @@ Psycopg 3.2.11 (unreleased)
   the Python version (:ticket:`#1153`).
 - Don't raise warning, and don't leak resources, if a builtin function is used
   as JSON dumper/loader function (:ticket:`#1165`).
+- Improve performance of Python conversion on results loading (:ticket:`#1155`).
 
 
 Current release

diff --git a/psycopg_c/psycopg_c/_psycopg/transform.pyx b/psycopg_c/psycopg_c/_psycopg/transform.pyx
index 6f81f1f5b73dba15b502f07893a3dcf2573e0fef..8b4c040990c32b7a6f0af086cc4a59731ba35172 100644 (file)
--- a/psycopg_c/psycopg_c/_psycopg/transform.pyx
+++ b/psycopg_c/psycopg_c/_psycopg/transform.pyx
@@ -443,36 +443,23 @@ cdef class Transformer:
         cdef object record  # not 'tuple' as it would check on assignment
 
         cdef object records = PyList_New(row1 - row0)
-        for row in range(row0, row1):
-            record = PyTuple_New(self._nfields)
-            Py_INCREF(record)
-            PyList_SET_ITEM(records, row - row0, record)
 
         cdef PyObject *loader  # borrowed RowLoader
-        cdef PyObject *brecord  # borrowed
+
         row_loaders = self._row_loaders  # avoid an incref/decref per item
 
-        for col in range(self._nfields):
-            loader = PyList_GET_ITEM(row_loaders, col)
-            if (<RowLoader>loader).cloader is not None:
-                for row in range(row0, row1):
-                    brecord = PyList_GET_ITEM(records, row - row0)
-                    attval = &(ires.tuples[row][col])
-                    if attval.len == -1:  # NULL_LEN
-                        pyval = None
-                    else:
+        for row in range(row0, row1):
+            record = PyTuple_New(self._nfields)
+
+            for col in range(self._nfields):
+                attval = &(ires.tuples[row][col])
+                if attval.len == -1:  # NULL_LEN
+                    pyval = None
+                else:
+                    loader = PyList_GET_ITEM(row_loaders, col)
+                    if (<RowLoader>loader).cloader is not None:
                         pyval = (<RowLoader>loader).cloader.cload(
                             attval.value, attval.len)
-
-                    Py_INCREF(pyval)
-                    PyTuple_SET_ITEM(<object>brecord, col, pyval)
-
-            else:
-                for row in range(row0, row1):
-                    brecord = PyList_GET_ITEM(records, row - row0)
-                    attval = &(ires.tuples[row][col])
-                    if attval.len == -1:  # NULL_LEN
-                        pyval = None
                     else:
                         b = PyMemoryView_FromObject(
                             ViewBuffer._from_buffer(
@@ -481,17 +468,16 @@ cdef class Transformer:
                         pyval = PyObject_CallFunctionObjArgs(
                             (<RowLoader>loader).loadfunc, <PyObject *>b, NULL)
 
-                    Py_INCREF(pyval)
-                    PyTuple_SET_ITEM(<object>brecord, col, pyval)
+                Py_INCREF(pyval)
+                PyTuple_SET_ITEM(record, col, pyval)
 
-        if make_row is not tuple:
-            for i in range(row1 - row0):
-                brecord = PyList_GET_ITEM(records, i)
+            if make_row is not tuple:
                 record = PyObject_CallFunctionObjArgs(
-                    make_row, <PyObject *>brecord, NULL)
-                Py_INCREF(record)
-                PyList_SET_ITEM(records, i, record)
-                Py_DECREF(<object>brecord)
+                    make_row, <PyObject *>record, NULL)
+
+            Py_INCREF(record)
+            PyList_SET_ITEM(records, row - row0, record)
+
         return records
 
     def load_row(self, int row, object make_row) -> Row: