Skip to content

Commit 8d3e3c2

Browse files
committed
Fix ~6x regression in numpy columnar LIST/ARRAY conversion
The NumpyArray facade read the buffer pointer via numpy's `ctypes.data` attribute chain and allocated via `numpy.empty(count, dtype_string)`. For a top-level column that runs once per 2048-row chunk (amortized), but the LIST/ARRAY per-element converter allocates a fresh array per row, so at 200k rows it became ~600k ctypes-object allocations: df()/fetchnumpy() of a LIST column ran ~6x slower than the pybind11 baseline (829ms vs 136ms). Read the buffer pointer directly from numpy's PyArrayObject C struct (a plain field read, as pybind11's array.data() did), gated by a PyObject_TypeCheck against numpy.ndarray so non-ndarray wrappers are never reinterpreted. Cache the numpy.empty callable and per-dtype np.dtype objects, and skip the no-op resize-to-current-length on the per-element path. Output is byte-identical (lists, nested, nulls, empty, masked, large-N); the row and arrow paths and the int/double/struct columnar paths are unaffected. LIST df()/fetchnumpy() now match-or-beat the pybind11 baseline (69ms).
1 parent 2983c92 commit 8d3e3c2

1 file changed

Lines changed: 83 additions & 19 deletions

File tree

src/include/duckdb_python/numpy/numpy_array.hpp

Lines changed: 83 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,58 @@
1111
#include "duckdb_python/nb/casters.hpp"
1212
#include "duckdb.hpp"
1313

14+
#include <unordered_map>
15+
1416
namespace duckdb {
1517

18+
namespace numpy_internal {
19+
20+
//! Mirror of the leading fields of numpy's `PyArrayObject` (stable ABI across numpy 1.x and 2.x).
21+
//! Only the buffer pointer is needed. Reading `data` is a plain struct field access -- no Python
22+
//! call, no allocation, no GIL -- exactly what pybind11's `py::array::data()` did internally via
23+
//! its own equivalent proxy struct. Obtaining the pointer this way (instead of via a `ctypes.data`
24+
//! attribute chain) is what keeps the numpy columnar path fast for LIST/ARRAY columns, whose
25+
//! per-element converter allocates a fresh array per row.
26+
struct NumpyArrayProxy {
27+
PyObject_HEAD
28+
char *data;
29+
};
30+
31+
//! Borrowed handle to the `numpy.ndarray` type, fetched once under the GIL and intentionally leaked
32+
//! for process lifetime (numpy is never unloaded). Used to gate the data-pointer read: the façade
33+
//! may also wrap non-ndarray objects (e.g. a pandas Index) whose buffer pointer is never read; for
34+
//! those the read must be skipped so a foreign object is never reinterpreted as a numpy array.
35+
inline PyTypeObject *NumpyNdarrayType() {
36+
static PyTypeObject *cached = []() -> PyTypeObject * {
37+
nb::object ndarray = nb::module_::import_("numpy").attr("ndarray");
38+
return reinterpret_cast<PyTypeObject *>(ndarray.release().ptr());
39+
}();
40+
return cached;
41+
}
42+
43+
//! Allocate an uninitialized 1-D numpy array of `count` elements with the given numpy dtype string.
44+
//! The bound `numpy.empty` and the `np.dtype` objects (a handful of distinct dtype strings) are
45+
//! cached to avoid a module import, an attribute lookup, and a dtype-string parse on every call --
46+
//! this is hot, since a LIST/ARRAY column allocates one array per row (pybind11 constructed the
47+
//! array at the C level via `py::array(py::dtype, count)`, which paid none of that). Cached handles
48+
//! are leaked for process lifetime (shutdown-safe: no Python destructor runs after finalization).
49+
//! Only ever called on the single-threaded, GIL-held result-materialization path.
50+
inline nb::object NumpyEmpty(idx_t count, const string &dtype) {
51+
static PyObject *empty_fn = []() -> PyObject * {
52+
nb::object fn = nb::module_::import_("numpy").attr("empty");
53+
return fn.release().ptr();
54+
}();
55+
static auto &dtype_cache = *new std::unordered_map<string, PyObject *>();
56+
PyObject *&descr = dtype_cache[dtype];
57+
if (!descr) {
58+
nb::object d = nb::module_::import_("numpy").attr("dtype")(dtype);
59+
descr = d.release().ptr();
60+
}
61+
return nb::borrow<nb::object>(empty_fn)(count, nb::handle(descr));
62+
}
63+
64+
} // namespace numpy_internal
65+
1666
//! Thin façade over the numpy array representation.
1767
//!
1868
//! This class is the SINGLE place in the codebase that owns the underlying numpy-array
@@ -21,15 +71,15 @@ namespace duckdb {
2171
//!
2272
//! Performance note: `Data()`/`MutableData()` are on the HOT path — the numpy scan calls
2373
//! `Data()` once per column per 2048-row chunk (see numpy_scan.cpp), and DuckDB drives that
24-
//! scan from multiple threads WITHOUT holding the GIL. Fetching the buffer address via
25-
//! `arr.ctypes.data` is ~1-5µs, allocates a numpy `_ctypes` object, and *requires the GIL*,
26-
//! so doing it per chunk would be both a scaling regression and a correctness hazard under a
27-
//! parallel scan. We therefore compute the pointer ONCE, eagerly, in the constructor (always
28-
//! invoked single-threaded with the GIL held at bind/result time) and cache it; `Data()` then
29-
//! becomes a plain pointer read with no Python call and no GIL — matching pybind11's
30-
//! `nb::array.data()`. The cache is invalidated (and recomputed) by `Resize()`, the only
31-
//! operation that reallocates the buffer. `ctypes.data` is also dtype-agnostic (works for the
32-
//! `object` dtype that DLPack/`nb::ndarray` cannot represent).
74+
//! scan from multiple threads WITHOUT holding the GIL. It is also on the LIST/ARRAY result path,
75+
//! where a fresh array (and thus a fresh buffer pointer) is materialized per row. The pointer is
76+
//! read directly from the numpy array's C struct (see `numpy_internal::NumpyArrayProxy`): a plain
77+
//! field access, no Python call, no allocation, no GIL — exactly what pybind11's
78+
//! `py::array::data()` did. We compute it ONCE, eagerly, in the constructor (always invoked
79+
//! single-threaded with the GIL held at bind/result time) and cache it; the cache is invalidated
80+
//! (and recomputed) by `Resize()`, the only operation that reallocates the buffer. Reading the
81+
//! struct field is dtype-agnostic (works for the `object` dtype that DLPack/`nb::ndarray` cannot
82+
//! represent).
3383
//!
3484
//! Ownership is move-only-when-asked: the ctor takes by value and moves, GetArray() hands
3585
//! back a reference, and no method copies the array buffer. The raw `cached_data_` member uses
@@ -54,8 +104,9 @@ class NumpyArray {
54104
//! dtype string (e.g. "int64", "float32", "object", "datetime64[us]"). Uninitialized —
55105
//! callers fill it immediately, matching the previous `nb::array(nb::dtype(d), count)`.
56106
static NumpyArray Allocate(const string &dtype, idx_t count) {
57-
auto numpy = nb::module_::import_("numpy");
58-
return NumpyArray(numpy.attr("empty")(count, dtype));
107+
NumpyArray result(numpy_internal::NumpyEmpty(count, dtype));
108+
result.length_ = count;
109+
return result;
59110
}
60111

61112
//! Produce a numpy array from an arbitrary Python object (np.asarray semantics: no copy
@@ -77,9 +128,16 @@ class NumpyArray {
77128

78129
//! Resize the underlying numpy buffer in place. This REALLOCATES the buffer, so the cached
79130
//! pointer is invalidated and recomputed (GIL is held -- this only runs on the single-threaded
80-
//! result-materialization path).
131+
//! result-materialization path). Resizing to the current length is a genuine no-op in numpy;
132+
//! we skip the Python `resize` call entirely in that case (buffer and cached pointer unchanged).
133+
//! The LIST/ARRAY per-element path allocates each array at its exact final size, so its
134+
//! `ToArray()` shrink-to-count is always such a no-op -- hot, hence worth skipping.
81135
void Resize(idx_t count) {
136+
if (length_ != DConstants::INVALID_INDEX && count == length_) {
137+
return;
138+
}
82139
array.attr("resize")(count, nb::arg("refcheck") = false);
140+
length_ = count;
83141
cached_data_ = nullptr;
84142
EnsurePointer();
85143
}
@@ -95,21 +153,27 @@ class NumpyArray {
95153

96154
private:
97155
//! Compute and cache the buffer start address of the underlying numpy array, if not already
98-
//! cached and an array is held. `ctypes.data` is dtype-agnostic (works for the `object` dtype
99-
//! too). Only ever called with the GIL held (construction / Resize).
156+
//! cached and a numpy ndarray is held. The pointer is read directly from the array's C struct
157+
//! (dtype-agnostic, works for the `object` dtype too), matching pybind11's `py::array::data()`.
158+
//! Only ever called with the GIL held (construction / Resize).
100159
void EnsurePointer() {
101-
// Only numpy ndarrays expose `ctypes`; some NumpyArray wrappers hold other objects (e.g. a pandas Index)
102-
// whose buffer pointer is never read. Guard the eager compute so constructing such a wrapper doesn't raise
103-
// (the original lazy code only touched `ctypes` if Data()/MutableData() was actually called).
104-
if (!cached_data_ && array.ptr() != nullptr && nb::hasattr(array, "ctypes")) {
105-
cached_data_ = reinterpret_cast<void *>(nb::cast<uintptr_t>(array.attr("ctypes").attr("data")));
160+
// Some NumpyArray wrappers hold non-ndarray objects (e.g. a pandas Index) whose buffer pointer is never read.
161+
// Gate the read on an actual numpy ndarray so we never reinterpret a foreign object's memory as an array.
162+
if (!cached_data_ && array.ptr() != nullptr &&
163+
PyObject_TypeCheck(array.ptr(), numpy_internal::NumpyNdarrayType())) {
164+
cached_data_ = reinterpret_cast<numpy_internal::NumpyArrayProxy *>(array.ptr())->data;
106165
}
107166
}
108167

109168
//! The owned numpy array (formerly `nb::array`).
110169
nb::object array;
111170
//! Cached buffer start address; see the class-level performance note.
112171
void *cached_data_ = nullptr;
172+
//! Known current element count, tracked so `Resize()` can skip a no-op. Set by `Allocate()` and
173+
//! updated by `Resize()`; `INVALID_INDEX` means "unknown" (arrays wrapped from arbitrary objects),
174+
//! in which case `Resize()` never skips. The array is only ever resized through `Resize()`, so
175+
//! this never goes stale.
176+
idx_t length_ = DConstants::INVALID_INDEX;
113177
};
114178

115179
} // namespace duckdb

0 commit comments

Comments
 (0)