Skip to content

Commit 40c1b32

Browse files
committed
trim
1 parent 38eaa4c commit 40c1b32

14 files changed

Lines changed: 49 additions & 65 deletions

File tree

src/include/duckdb_python/nb/conversions/enum_string_caster.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
value = FromIntegerFn(nanobind::cast<int64_t>(src)); \
4040
return true; \
4141
} \
42-
/* Registered nb::enum_ instances aren't int subclasses (unlike pybind11's), so accept a member */ \
42+
/* Registered nb::enum_ instances aren't int subclasses, so accept a member */ \
4343
/* of the registered enum by reading its integer .value. */ \
4444
nanobind::handle enum_type = nanobind::type<EnumType>(); \
4545
if (enum_type.is_valid() && PyObject_IsInstance(src.ptr(), enum_type.ptr()) == 1) { \

src/include/duckdb_python/numpy/numpy_array.hpp

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,11 @@ namespace duckdb {
1818
namespace numpy_internal {
1919

2020
//! 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.
21+
//! Reading `data` is a plain struct field access (no Python call, allocation, or GIL). Obtaining
22+
//! the pointer this way, instead of via a `ctypes.data` attribute chain, keeps the numpy columnar
23+
//! path fast for LIST/ARRAY columns, whose per-element converter allocates a fresh array per row.
2624
struct NumpyArrayProxy {
27-
PyObject_HEAD
28-
char *data;
25+
PyObject_HEAD char *data;
2926
};
3027

3128
//! Borrowed handle to the `numpy.ndarray` type, fetched once under the GIL and intentionally leaked
@@ -41,12 +38,10 @@ inline PyTypeObject *NumpyNdarrayType() {
4138
}
4239

4340
//! 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.
41+
//! `numpy.empty` and the `np.dtype` objects are cached to avoid a module import, attribute lookup,
42+
//! and dtype-string parse on every call. This is hot: a LIST/ARRAY column allocates one array per
43+
//! row. Cached handles are leaked for process lifetime (shutdown-safe: no Python destructor runs
44+
//! after finalization). Only ever called on the single-threaded, GIL-held result path.
5045
inline nb::object NumpyEmpty(idx_t count, const string &dtype) {
5146
static PyObject *empty_fn = []() -> PyObject * {
5247
nb::object fn = nb::module_::import_("numpy").attr("empty");
@@ -69,17 +64,15 @@ inline nb::object NumpyEmpty(idx_t count, const string &dtype) {
6964
//! object. Under nanobind there is no `nb::array` (and no `nb::dtype`); the array is held
7065
//! as a plain `nb::object` and the few buffer operations go through numpy directly.
7166
//!
72-
//! Performance note: `Data()`/`MutableData()` are on the HOT path — the numpy scan calls
73-
//! `Data()` once per column per 2048-row chunk (see numpy_scan.cpp), and DuckDB drives that
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).
67+
//! Performance note: `Data()`/`MutableData()` are on the HOT path. The numpy scan calls `Data()`
68+
//! once per column per 2048-row chunk (see numpy_scan.cpp), and DuckDB drives that scan from
69+
//! multiple threads WITHOUT holding the GIL. It is also on the LIST/ARRAY result path, where a
70+
//! fresh array (and buffer pointer) is materialized per row. The pointer is read directly from the
71+
//! numpy array's C struct (see `numpy_internal::NumpyArrayProxy`): a plain field access, no Python
72+
//! call, allocation, or GIL. We compute it ONCE, eagerly, in the constructor (single-threaded with
73+
//! the GIL held at bind/result time) and cache it; the cache is invalidated (and recomputed) by
74+
//! `Resize()`, the only operation that reallocates the buffer. The struct read is dtype-agnostic
75+
//! (works for the `object` dtype that DLPack/`nb::ndarray` cannot represent).
8376
//!
8477
//! Ownership is move-only-when-asked: the ctor takes by value and moves, GetArray() hands
8578
//! back a reference, and no method copies the array buffer. The raw `cached_data_` member uses
@@ -101,8 +94,8 @@ class NumpyArray {
10194

10295
public:
10396
//! Allocate a fresh, contiguous 1-D numpy array of `count` elements with the given numpy
104-
//! dtype string (e.g. "int64", "float32", "object", "datetime64[us]"). Uninitialized
105-
//! callers fill it immediately, matching the previous `nb::array(nb::dtype(d), count)`.
97+
//! dtype string (e.g. "int64", "float32", "object", "datetime64[us]"). Uninitialized; callers
98+
//! fill it immediately.
10699
static NumpyArray Allocate(const string &dtype, idx_t count) {
107100
NumpyArray result(numpy_internal::NumpyEmpty(count, dtype));
108101
result.length_ = count;
@@ -127,11 +120,11 @@ class NumpyArray {
127120
}
128121

129122
//! Resize the underlying numpy buffer in place. This REALLOCATES the buffer, so the cached
130-
//! pointer is invalidated and recomputed (GIL is held -- this only runs on the single-threaded
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.
123+
//! pointer is invalidated and recomputed (GIL held; only runs on the single-threaded result
124+
//! path). Resizing to the current length is a genuine no-op in numpy, so we skip the Python
125+
//! `resize` call entirely in that case. The LIST/ARRAY per-element path allocates each array at
126+
//! its exact final size, so its `ToArray()` shrink-to-count is always such a no-op: hot, worth
127+
//! skipping.
135128
void Resize(idx_t count) {
136129
if (length_ != DConstants::INVALID_INDEX && count == length_) {
137130
return;
@@ -143,7 +136,7 @@ class NumpyArray {
143136
}
144137

145138
//! Access the underlying array, e.g. for `.attr(...)` calls, iteration, or to hand it
146-
//! back to Python. Returned by reference -- never copied.
139+
//! back to Python. Returned by reference, never copied.
147140
nb::object &GetArray() {
148141
return array;
149142
}
@@ -154,8 +147,8 @@ class NumpyArray {
154147
private:
155148
//! Compute and cache the buffer start address of the underlying numpy array, if not already
156149
//! 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).
150+
//! (dtype-agnostic, works for the `object` dtype too). Only ever called with the GIL held
151+
//! (construction / Resize).
159152
void EnsurePointer() {
160153
// Some NumpyArray wrappers hold non-ndarray objects (e.g. a pandas Index) whose buffer pointer is never read.
161154
// Gate the read on an actual numpy ndarray so we never reinterpret a foreign object's memory as an array.

src/native/python_conversion.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ void TransformPythonObjectInternal(optional_ptr<ClientContext> context, nb::hand
10431043
}
10441044
case PythonObjectType::Bytes: {
10451045
// Read the buffer directly (mirrors the ByteArray branch above): nanobind's nb::cast<std::string> rejects
1046-
// a bytes object (pybind11 accepted it), so go through the CPython API instead.
1046+
// a bytes object, so go through the CPython API instead.
10471047
char *bytes_buffer;
10481048
Py_ssize_t bytes_length;
10491049
PyBytes_AsStringAndSize(ele.ptr(), &bytes_buffer, &bytes_length); // NOLINT

src/native/python_objects.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -663,13 +663,6 @@ nb::object PythonObject::FromValue(const Value &val, const LogicalType &type,
663663
auto array_size = ArrayType::GetSize(type);
664664
auto &child_type = ArrayType::GetChildType(type);
665665

666-
// do not remove the static cast here, it's required for building
667-
// duckdb-python with Emscripten.
668-
//
669-
// without this cast, a static_assert fails in pybind11
670-
// because the return type of ArrayType::GetSize is idx_t,
671-
// which is typedef'd to uint64_t and ssize_t is 4 bytes with Emscripten
672-
// and pybind11 requires that the input be castable to ssize_t
673666
duckdb::PyUtil::TupleBuilder arr(array_size);
674667
for (idx_t elem_idx = 0; elem_idx < array_size; elem_idx++) {
675668
arr.append(FromValue(array_values[elem_idx], child_type, client_properties));

src/pandas/analyzer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ LogicalType PandasAnalyzer::DictToStruct(const PyDictionary &dict, bool &can_con
339339

340340
//! Have to already transform here because the child_list needs a string as key. Stringify via str() so
341341
//! non-string keys (e.g. the integer keys of a hashable-key MAP, produced as a plain {1: 10} dict) are
342-
//! accepted -- nanobind's nb::cast<std::string> rejects non-str objects, whereas pybind11 stringified them.
342+
//! accepted (nb::cast<std::string> rejects non-str objects).
343343
auto key = Identifier(nb::cast<std::string>(nb::str(dict_key)));
344344

345345
auto dict_val = dict.values.attr("__getitem__")(i);

src/pyconnection.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ DuckDBPyConnection::~DuckDBPyConnection() {
6969
// the GIL for it so other Python threads can run. The implicit member
7070
// destructors that fire after this scope (notably
7171
// `registered_functions`, a `case_insensitive_map_t<unique_ptr<ExternalDependency>>`
72-
// whose entries transitively own pybind-managed Python references)
72+
// whose entries transitively own Python references)
7373
// run with the GIL reacquired because `gil` is destroyed at the end
7474
// of the inner block.
7575
{
@@ -477,7 +477,7 @@ DuckDBPyConnection::RegisterScalarUDF(const string &name, const nb::callable &ud
477477
}
478478

479479
void DuckDBPyConnection::Initialize(nb::handle &m) {
480-
// Weak-referenceable like pybind11 (which set tp_weaklistoffset by default); nanobind requires the opt-in,
480+
// nanobind types aren't weak-referenceable by default;
481481
// otherwise weakref.ref/proxy/finalize on a connection raises TypeError.
482482
auto connection_module = nb::class_<DuckDBPyConnection>(m, "DuckDBPyConnection", nb::is_weak_referenceable());
483483

@@ -1920,7 +1920,7 @@ void DuckDBPyConnection::Close() {
19201920
// is pure C++ work and can take noticeable time. Hold the GIL back for
19211921
// `registered_functions.clear()` because the
19221922
// `case_insensitive_map_t<unique_ptr<ExternalDependency>>` it destroys
1923-
// transitively owns pybind-managed Python references (Python UDF
1923+
// transitively owns Python references (Python UDF
19241924
// callables, registered Python objects, …). Decrementing those
19251925
// references with the GIL released is undefined behaviour — see
19261926
// duckdb-python#456.
@@ -2166,7 +2166,7 @@ duckdb::pyarrow::RecordBatchReader DuckDBPyConnection::FetchRecordBatchReader(co
21662166
case_insensitive_map_t<Value> TransformPyConfigDict(const nb::dict &py_config_dict) {
21672167
case_insensitive_map_t<Value> config_dict;
21682168
for (auto kv : py_config_dict) {
2169-
// Config values may be int/bool/str; str-ify them (matches pybind11's nb::str(value)) rather than
2169+
// Config values may be int/bool/str; str-ify them rather than
21702170
// requiring an actual Python str (nb::cast<std::string> would throw on a non-str like 0 or False).
21712171
auto key = nb::cast<std::string>(nb::str(kv.first));
21722172
auto val = nb::cast<std::string>(nb::str(kv.second));

src/pyexpression.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ bool DuckDBPyExpression::TryToExpression(nb::handle obj, std::unique_ptr<DuckDBP
353353
// A str becomes a column reference, mirrors the registered str constructor.
354354
result = ColumnExpression(nb::cast<nb::args>(nb::make_tuple(obj)));
355355
} else if (nb::isinstance<nb::bytes>(obj)) {
356-
// pybind11 decoded bytes as UTF-8 and (like str) treated them as a column reference; preserve that
356+
// Decode bytes as UTF-8 and treat like str (a column reference),
357357
// so e.g. rel.project(b"col") references column "col" instead of silently building a BLOB constant.
358358
result = ColumnExpression(nb::cast<nb::args>(nb::make_tuple(obj.attr("decode")("utf-8"))));
359359
} else {

src/pyexpression/initialize.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ namespace {
1111
// Binary operators take their operand as nb::object (not Expression) so that None can bind: nanobind rejects None for a
1212
// bound-type parameter before the registered implicit conversion runs, so `expr == None` / `expr + None` would never
1313
// reach the None -> SQL NULL conversion otherwise. We convert explicitly via TryToExpression (an existing Expression is
14-
// copied, a str becomes a column reference, any other value -- including None -- becomes a constant). On a genuinely
14+
// copied, a str becomes a column reference, any other value (including None) becomes a constant). On a genuinely
1515
// unconvertible operand we return Py_NotImplemented so Python falls back to the reflected operator / identity
16-
// comparison, exactly as the is_operator() overload did under pybind11 (keeps e.g. `expr == object()` returning False
16+
// comparison, keeping e.g. `expr == object()` returning False
1717
// instead of raising).
1818
template <typename Build>
1919
nb::object ExpressionBinaryOp(const nb::object &other, Build &&build) {
@@ -325,7 +325,7 @@ static void InitializeImplicitConversion(nb::class_<DuckDBPyExpression> &m) {
325325
}
326326

327327
void DuckDBPyExpression::Initialize(nb::module_ &m) {
328-
// Weak-referenceable like pybind11 (nanobind requires the explicit opt-in).
328+
// nanobind types aren't weak-referenceable by default.
329329
auto expression = nb::class_<DuckDBPyExpression>(m, "Expression", nb::is_weak_referenceable());
330330

331331
InitializeStaticMethods(m);

src/pyrelation/initialize.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ static void InitializeMetaQueries(nb::class_<DuckDBPyRelation> &m) {
278278
}
279279

280280
void DuckDBPyRelation::Initialize(nb::handle &m) {
281-
// Weak-referenceable like pybind11 (nanobind requires the explicit opt-in).
281+
// nanobind types aren't weak-referenceable by default.
282282
auto relation_module = nb::class_<DuckDBPyRelation>(m, "DuckDBPyRelation", nb::is_weak_referenceable());
283283
InitializeReadOnlyProperties(relation_module);
284284
InitializeAggregates(relation_module);

src/pyresult.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ DuckDBPyResult::DuckDBPyResult(unique_ptr<QueryResult> result_p) : result(std::m
3737

3838
DuckDBPyResult::~DuckDBPyResult() {
3939
// The destructor must run with the GIL held: `result` and `current_chunk`
40-
// can transitively own pybind-managed Python references (registered
40+
// can transitively own Python references (registered
4141
// objects, arrow release callbacks, PYTHON_OBJECT vector values, etc.),
4242
// whose teardown calls into the Python C API. Releasing the GIL here
4343
// (as the previous implementation did) causes Py_DECREF / PyObject_Free

0 commit comments

Comments
 (0)