Skip to content

Native: run_in_transaction takes the commit-count witness from the begin reply (4 to 3 crossings) - #201

Merged
tae898 merged 2 commits into
mainfrom
perf/native-tx-crossings
Nov 3, 2026
Merged

tae898 merged 2 commits into
mainfrom
perf/native-tx-crossings

Conversation

@tae898

@tae898 tae898 commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Port of humemai/arcadedb-embedded-python#522 (branch perf/native-tx-crossings, closed there when the bindings moved, never merged) onto current main. Refs #19. Opened as a draft: a timed A/B decides it before merge.

NativeDatabase.run_in_transaction made 4 library crossings on the happy path (_transaction_stack_mark, begin, _transaction_commit_mark, commit), each one a ctypes call, a malloc'd JSON reply, a decode and a json.loads, although the begin reply already carries {"active","depth","commits"} and begin() discarded it. The commit-count witness now comes from the begin reply (new internal _begin_state(); public begin() still returns None). The unwind after a failed block asked is_transaction_active() before every rollback and once more at the end, and the retry check asked for the commit count again; every rollback reply carries the same state, so the new _unwind_opened_since(mark) returns the state of its last crossing and the retry check reads the commit count from it. A crossing that failed leaves the state unknown and the next step reads it again, as before. _end_transactions_opened_since(mark) keeps its signature and behaviour. Python only, no Java change, no image rebuild.

Counts (lib.call invocations, tests/test_native_tx_crossings.py, native only and not collected on JPype): run_in_transaction happy path 4 to 3 (mark, begin, commit); a block that raises 6 to 4; a refused attempt that is retried and then commits 11 to 7; with db.transaction() stays at 3. Two crossings is not reachable from Python alone: the mark has to be taken before begin (a mark taken after it counts the level begin pushed as the caller's), so it is a crossing of its own. Semantics are unchanged by design and pinned by tests that pass before and after (a nested block that fails leaves the caller's transaction open, a refused attempt is rolled back before the retry, a duplicated key is retried once, an error without a retry kind is raised after one attempt). The P-03 guard (the witness check raises outside its try) is already on main and is kept.

tests/test_interrupt_hygiene.py patched Database.begin and Database.rollback, which the native run_in_transaction no longer calls, so on native the patches would never apply. The tests now patch the seam the active backend's run_in_transaction really calls (begin and rollback on the JPype Database, _begin_state and _rollback_state on the native one). The one test with no native equivalent (an interrupt between begin and a separate commit-mark read, which the begin reply made one crossing) is removed on native instead of skipped, because scripts/check_test_skips.py fails a native run on any skip. JVM behaviour is untouched.

No speed claim is made. Whether 1 of 4 crossings shows in a small-transaction rate is decided by a timed A/B (tools/ab.py, laptop, main's native wheel against this PR's wheel) before this leaves draft. Draft PRs skip the test-native.yml legs, so the PR's wheel and the parity run come from gh workflow run test-native.yml --ref perf/native-tx-crossings. Checks run locally on the changed files: compile, pyflakes (only the findings main already has), black. The tests themselves have not been run on this port; the parity run is the first run.

🤖 Generated with Claude Code

…gin reply, and the unwind reads no state it already has

Port of humemai/arcadedb-embedded-python#522 (branch perf/native-tx-crossings, never merged there) onto current main; the P-03 guard of that PR's base is already here.

run_in_transaction made 4 library crossings on the happy path (mark, begin, commit mark, commit) although the begin reply already carries {"active","depth","commits"}: the witness now comes from that reply, 3 crossings (the mark must stay before begin). The unwind after a failed block asked is_transaction_active() before every rollback and once more at the end, and the retry check asked for the commit count again; every rollback reply carries the same state, so the unwind returns the state of its last crossing and the retry check reads the commit count from it (a failed crossing falls back to a read). Failed block 6 -> 4 crossings, refused and retried attempt 11 -> 7.

begin() and rollback() stay public and return None; _begin_state() and _rollback_state() are the seams run_in_transaction calls. test_interrupt_hygiene.py patched begin and rollback, which the native run_in_transaction no longer calls, so it patches the seam the active backend calls. test_native_tx_crossings.py counts the arcadedb_n_tx operations and pins the behaviour the marks exist for. Counts only, no speed claim: a timed A/B decides it before merge.

Refs #19

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tae898

tae898 commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Timed A/B of this PR against main (laptop timed lane, ab.py --noise, ABBA pairs, an A/A control; the rule was written in the study before the first run). Both arms use main's native wheel; arm B swaps in this PR's database.py and records.py, so libarcadedb.so is the same file in both.

figure role clean pairs median PR / main 95% CI
rit_empty_us headline 29 0.773 [0.770, 0.777]
rit_insert1_us headline 29 0.988 [0.982, 0.993]
rit_raise_us headline 29 0.715 [0.711, 0.717]
rit_insert10_us target 29 0.993 [0.990, 0.997]
ctl_with_tx_us guard 29 1.003 [0.994, 1.004]
ctl_first_us guard 29 0.993 [0.989, 1.001]

Result: ok (all 6 B/A figures have >= 10 clean pairs and CI width <= 0.08, the A/A control is clean). By the rule the result was GRAY at round 12 (the one-insert transaction gains less than the 3 % the rule asks of every headline figure), so it was extended to round 30 as the rule says; still GRAY there, the median decides: every headline and target median favours the PR and the guards did not move, so it ships. In short: the fixed cost of run_in_transaction drops by about a quarter (empty and raising blocks), which is about 1 % of a transaction that inserts a record.

@tae898
tae898 marked this pull request as ready for review November 3, 2026 01:13
…both native-only test ignores)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tae898
tae898 merged commit e1f5499 into main Nov 3, 2026
38 checks passed
@tae898
tae898 deleted the perf/native-tx-crossings branch November 3, 2026 03:23
tae898 added a commit that referenced this pull request Nov 3, 2026
Conflicts resolved:
- src/arcadedb/_native/records.py: keeps this branch's _save_update (lazy fingerprint, refuses a stale overwrite) and applies #202's rule inside it: the record id goes to the engine as the :rid parameter for both the stored-record read (_SELECT = "SELECT FROM :rid") and the UPDATE .. MERGE .. RETURN AFTER (_UPDATE now carries RETURN AFTER, which only _save_update uses). Without this, #202's test_native_statement_texts fails on the pasted ids in the save path.
- tests/conftest.py, tests/test_graph_api.py: both sides appended; kept both. The duplicate `import pytest` from the two sides is dropped and main's import order kept (local isort disagrees with main's CI order; left as main has it).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tae898 added a commit that referenced this pull request Nov 3, 2026
No textual conflicts: main's #202 (record ids as a :rid parameter, src/arcadedb/_native/records.py and database.py) and #201 (run_in_transaction witness) touch other hunks than this branch. The record-id rule of #202 is kept: this branch pastes no record id into SQL text (the lazy per-hit vector load, if present, goes through lookup_by_rid, which keeps its literal id by design, see tests/test_native_statement_texts.py).

Static checks after the merge: compile() and pyflakes of the changed files (the unused typing.Optional in records.py is on main too), pre-commit black and isort pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tae898 added a commit that referenced this pull request Nov 3, 2026
No textual conflicts: main's #202 (record ids as a :rid parameter, src/arcadedb/_native/records.py and database.py) and #201 (run_in_transaction witness) touch other hunks than this branch. The record-id rule of #202 is kept: this branch pastes no record id into SQL text (the lazy per-hit vector load, if present, goes through lookup_by_rid, which keeps its literal id by design, see tests/test_native_statement_texts.py).

Static checks after the merge: compile() and pyflakes of the changed files (the unused typing.Optional in records.py is on main too), pre-commit black and isort pass.

Co-Authored-By: Claude Opus 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