Skip to content

Native: read each library reply from its address, one copy, freed once (closes #323) - #327

Merged
tae898 merged 1 commit into
release/26.11.1from
perf/323-native-reply-owner
Nov 10, 2026
Merged

tae898 merged 1 commit into
release/26.11.1from
perf/323-native-reply-owner

Conversation

@tae898

@tae898 tae898 commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #323. Part of #315 and the glue changes of the wheels-only 26.11.1.post1. Nothing changes for users: every reply decodes to the same text and the same floats, and the same errors are raised.

What changes: the native wheel read each library reply with ctypes.string_at (a bytes copy), cut the float tail marker out of it (a second and third copy of the bytes), decoded it to str (another copy), and only then parsed it. It now decodes straight from the reply's address with PyUnicode_FromStringAndSize (strict UTF-8, the same UnicodeDecodeError as bytes.decode), so the text is copied once, into the str that json.loads reads. For a reply with a float32 tail only the last 40 bytes (to find ,"bin":N}) and the N floats are copied as bytes; the marker's comma is overwritten with } in the reply's own memory before the decode, so the slice and concatenation of the text are gone. The three readers (call, call_bin, exec_query) share one function, lib._take, which replaces two near-duplicate copies.

Lifetime: I did not make a garbage-collected owner object. A reply never outlives the call that takes it (the text becomes a str and the floats a copy before the call returns), so nothing can hold a view of the freed memory, and a finalizer would have to free from any thread or at interpreter exit, when the isolate thread handle may be gone. The free is in the finally of _take: exactly once, also when the decode or the tail parse raises. A NULL reply raises ArcadeDBError and frees nothing. The float tail is still one bytes copy because it must be read before the free; making it a view would need the owner object above.

Platforms: the reader needs the length of a NUL-terminated reply, and strlen comes from libc (ctypes.CDLL(None) on Linux and macOS) or msvcrt (ctypes.cdll.msvcrt on Windows) through ctypes, and the decode uses PyUnicode_FromStringAndSize from ctypes.pythonapi, a CPython C-API function. The new tests passed on the native parity legs for linux-x86_64, linux-aarch64 and windows-x64 (Python 3.12 and 3.14). PyPy and GraalPy were not checked: I do not know whether their ctypes.pythonapi offers that function, so on them the reader is unverified.

Peak memory does not rise: the bytes copy is gone and nothing new is held.

Tests (tests/test_native_reply_reader.py, native backend only, run in the native CI legs): each case (ASCII, non-ASCII with 2, 3 and 4 byte sequences, a 5 kB text, short texts, every alignment padding of the tail, a marker-like text deep in the reply) is read by _take and by a private copy of the former reader from two identical replies in the library's own memory, and the text, the floats and the pointer freed must agree. Spies check that a text reply makes no ctypes.string_at call at all (the former reader made one with the whole reply), that a reply with a tail asks only for 40 bytes and the floats (never the 5 kB text), and that arcadedb_free runs once with the pointer that was read, also for invalid UTF-8 (including an encoded surrogate), a bad tail count, and the public call and call_bin. A NULL reply is covered too (ArcadeDBError, no free), as is invalid UTF-8 in both the text-only and the tail form (three byte strings, one of them an encoded surrogate). The golden test tests/test_cross_wheel_golden.py is untouched. I could not run any of it here (CI only); the evidence is the runs listed below.

No speed claim: the measurement is a later laptop A/B, and the untimed profile studies queued for tonight.

🤖 Generated with Claude Code

…py, free it once (#323)

The reply was copied as bytes (string_at), cut for the float tail, decoded to str,
then parsed. It is now decoded straight from the library's memory with
PyUnicode_FromStringAndSize, only the last 40 bytes and the float tail are copied as
bytes, and the free is in the finally of the same call.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@tae898 tae898 self-assigned this Nov 9, 2026
@tae898
tae898 merged commit 34b64c6 into release/26.11.1 Nov 10, 2026
99 of 115 checks passed
@tae898
tae898 deleted the perf/323-native-reply-owner branch November 10, 2026 02:23
@github-actions github-actions Bot added this to the 26.12.1 milestone Nov 10, 2026
abhirajadhikary06 pushed a commit to abhirajadhikary06/arcadedb-python that referenced this pull request Nov 10, 2026
The release line (tag 26.11.1.post2 = 1454809, which contains the post1 tag
26.11.1.post1 = dbe1670) branched from the 26.11.1 tag and carries the post1
candidates (the native column frames ArcadeData#331, the other native and JVM load and read changes
of ArcadeData#327 to ArcadeData#339, and the cherry-picks of main's ArcadeData#320 and ArcadeData#321),
the post2 additions (Windows parity harness ArcadeData#295/ArcadeData#347, ArcadeData#340, the JVM Arrow
vector load change) and their test fixes. Main did not have them. The merge
uses a merge commit so that the release tag stays an ancestor of main.

Four files conflicted, each because main and the release line changed the
same lines. Resolutions, each checked against the history of both sides:

- src/arcadedb/_native/columns.py (modify/delete): deleted. Main changed it
  only with ArcadeData#321 (6c6a5a1). The release line holds the same change as the
  cherry-pick 946dfda (identical blobs of columns.py and of
  test_native_columns_reference.py at both commits) and then deleted the
  module with the native column frames (ArcadeData#331, d606cce). Nothing else imports
  it; the merged database.py reads columns through the frames code.
- src/arcadedb/_native/database.py: kept both. Main's facets lines in _fill
  and its facets property (ArcadeData#299, ArcadeData#301), then the release line's
  self._note_vectors(reply) before the return and its _note_vectors method.
- tests/conftest.py: the release side of the native-only block (the
  comment of test_native_columns_reference.py now says what the file
  compares, plus the two new native-only files), and both entries in
  WINDOWS_NOT_APPLICABLE (main's test_dev_engine_pin.py and the release
  line's test_parity_gate_awk.py).
- tests/test_native_columns_reference.py (add/add): the release side whole.
  Main's file tested the deleted module.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant