Repository navigation
Pine adapter: per-bar and per-fill walks read only the cohorts that hold an opening or live units, and the roster pass only the rosters with members (#452) - #458
Conversation
…4pass#452) Adds the uniqueids row to test_adapter_live_state_scaling: an opening every 20 bars under an id of its own, closed 10 bars later, without recording, the way scripts that tag every trade with its bar or time spell strategy.entry. The adapter keeps a cohort for every id the run has used and the kernel a roster for each, and walks over all of them ran on every bar and at every fill and placement, so the run cost the square of its trades. Fails at this commit (base 0212cab, macOS 15.8 arm64, Apple clang 17, Release): FAIL tests/test_adapter_live_state_scaling.cpp:476 ratio < kShapeBound ... 32000 bars 0.3703s (1600 trades, ...), 128000 bars 8.4500s (6400 trades, ...), ratio 22.82 (bound 5.0) Signed-off-by: letiep97 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
…e units, and the roster pass only rosters with members (pineforge-4pass#452) cohorts_by_id_ / cohort_order_ keep a cohort for every entry id the run has used, and the core a CohortRoster for each handle; neither is pruned, since origin history and the state hash read them. The walks that only read `opened` and `live_units_by_origin` (source_entry_slot_count every bar, the two in erase_retired_rows, and some twenty at fills and placements) now go over held_cohort_ids_: the ids whose cohort holds an opening or live units, in cohort_order_ order, which is handle order. hold_cohort() adds an id when its cohort gains an opening; forget_emptied_cohorts() drops ids whose cohort lost its last opening and live units. Every non-Release read asserts the index equals what a full walk finds, in the same order. release_closed_cohort_origins keeps, in retired_row_scratch_, the handles of the rosters that hold members, fed by the core's append-only cohort_receipts() (only an applied add puts a member in a roster), and its four loops read those rosters only; non-Release builds assert the count against a walk over every roster. Both are derived: held_cohort_ids_ gets a waiver in broker_state_hash_waivers.txt, and the retired_row_scratch_ waiver names the roster memo. Not covered: a fill that reverses the position still calls cancel_exit_orders_for_full_close for every cohort id the run has used (on_applied, flipped_position branch). uniqueids row after: 128000 bars 0.1011 s, 512000 bars 0.4149 s, ratio 4.11 (bound 5.0). The pineforge-4pass#452 script on 702,505 BTCUSDT 5m bars: 232.09 s -> 1.72 s, trades and equity identical field by field. doc anchors: every pine_adapter.cpp / pine_adapter.hpp path:line anchor moved to the line its symbol now sits on (docs/adr/0001-kernel-adapter- boundary.md, docs/design/native-feature-parity.md, docs/native-refactor-progress.md, docs/pages/native-engine.md, two tests/ownership probe NOTES.md, six tests/*.cpp comments, and three comments in pine_adapter.cpp itself). corpus_parity_mutation_battery.tsv is left alone: its lines name an older tree already at base. Signed-off-by: letiep97 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thank you for this pull request, and sorry that it has gone unanswered since 10-20. That was our miss, not yours. We are reading it now and will answer here, in detail, by 2026-10-27. |
|
We have now read it. In the code your change was written against, the per-bar and per-fill steps you listed walked every cohort the run had used, so their cost grew with the number of order ids used. Since then the way cohorts are stored on main was replaced, and most of those steps now walk only the rows kept in memory. At least one step still visits every id the run has used, whenever it runs: the pass over the kernel's rosters in the release step. From our reading of the code, that part of your report still appears to hold on main, and we are measuring it; it is what your second change addresses. Your reproducer (an opening every 20 bars under a new id, closed 10 bars later) looks like a good probe of it, and we would like to use it as a test, with credit. The idea of your first change, keeping a list of only the cohorts that hold an opening or live units, in the original order, looks sound to us from reading it, and we are checking it against the cohort storage on main. We will measure your second change, the roster pass, on its own and tell you the result. Because the storage changed, your change no longer applies and we cannot merge it as it is. We are working on the same costs on the new storage for a coming release, and we are building on your idea. We will credit you as a co-author on the commit that uses it, and we will tell you here which parts we take and which we do not. You are welcome to rebase your change onto the current main; if you do, we will review the rebased version on its own merits. When our work is merged we will comment here, and decide then, with you, whether to close this pull request. |
…g orders, live order-key guard, id admission and session fixes (#541) Part of #452. Credit: the "walk live cohorts only" shape follows the external proposal in #458. This is the engine side of the 1.8.0 integration: the cohort-table work for runs with many pending orders, the guard on live order keys with its optional run-failure argument, and four smaller fixes. It targets engine 1.8.0. ## Behaviour changes users can see - Jailed profile: an id longer than 64 bytes is refused before any store mutation with scratch_capacity_exceeded ("id longer than 64 bytes"). The jailed default scratch manifest follows a 64 MiB share (id table 262,144 / 32,768 / 65,536 entries), mirrored in docker/run_json.py. The resident floor of a jailed run rises by about 51 MiB (the slab is zero-filled at construction: 61,128,704 B computed, +52 MB measured as jailed peak RSS on a dense run), a fixed floor, not growth with bars. The jailed ceiling for distinct entry ids moves from 4,096 to 65,536. Unjailed runs are unchanged. - Two live order keys whose 64-bit hashes collide are refused with engine_invariant instead of one silently replacing the other. engine_invariant gains an optional vocabulary argument `reason` with the single value order_key_hash_collision; every other engine_invariant still carries no argument, no code is added or removed, and hosts should accept (not pin the absence of) the optional argument. The catalog diff shows one changed code. - Strategy ids, entry selectors and OCA names that contain NUL or the unit separator (0x1f) are refused at admission with pine_invalid_argument (rule unsupported_combination), because composite exit and replacement keys join two user strings with 0x1f. - A session string with more than eight windows is parsed completely (it stopped at the ninth window, so the daily close fell back to the first window only). Up to eight windows keep the fixed-size storage and allocate nothing. - The per-chart range vectors of the aux security feed give their capacity back at terminal points of a batch run. ## Cohort tables - A full close no longer seeks the scratch index once per owner for every bracket family: BracketRoster::remove_selected checks the in-memory incarnation counts first. A close on N owners goes from N(N+1)/2 seeks to 2N+1. On the dense shape the old code counted 8,385 and 33,153 find_key calls where the new code counts 257 and 513 (a test run, counts printed by the test, not seconds). - DiskResidentIdTable keeps an ordered held set of the resident rows that hold an opening or live units (held_rows()), so the walks that need only those rows no longer visit every pending-only cohort. Credit: the proposal in #458. - Resident cohort lookup is O(log R) without allocation or disk; the implicit spill is amortised; kMaxResidentCohorts = 4096 non-spillable cohorts, jailed profile only; refusal texts state the live manifest limit and a workaround. Measured on a release build of this change against engine main, on a 1m ETH-USDT feed, 4 repetitions per cell (2 at 60,000 and 100,000 bars), 600 s cap per run: the dense script at 4,000 bars goes from 83.9 s to 6.4 s jailed and from 85.8 s to 7.4 s unjailed; the same reports are produced (normalised report hashes equal in all 16 cells where both finished). At 10,000 bars and above the dense script does not finish within 600 s on either tree, so this change does not make the dense shape scale: the per-bar retire pass is still Theta(R) in the number of resident rows, and the close event is still Theta(N^2) in cheap in-memory work (only the scratch seeks became linear). A script with many orders and few ids is 13, 12.5 and 16 percent faster jailed at 30,000, 60,000 and 100,000 bars. ## Live order-key guard Before a live handle is selected or overwritten, the identity captured for the key is compared with the placement ids. Represented spellings are checked on insert, replace, lookup and erase; a spelling the table does not classify keeps the previous hash-only behaviour and never causes a refusal. A new test walks every represented spelling through a public run, and a check confirms the strict missing-spelling diagnostic text is absent from the built library archive in every build configuration. ## Source allocation gate The source-bounds gate gains entries for the new container sites: the unreviewed ceiling goes from 1,241 to 1,264 and the pinned OPEN count from 1,429 to 1,452. None of the new entries claims a bound; this is not a flat-memory proof. ## Proof The engine verification steps (preflight, release, debug, sanitizers, kernel, docs, parity, subset) all pass on this tree; parity measures 312 probes with 312 matching the baseline and none moved. The private-fixture rows of the touched tests, including test_run_failure_codes, pass with the fixtures overlaid. ## Replaced documentation text In docs/pages/scratch-tables.md two texts from commit 91353de are replaced on purpose by the new constants (they are not reverts): the sentence "The key/byte choices came from an admission policy using 4,096 roster entries and 256-byte components; they are aggregate descriptor limits, not a new per-string length check", and the arithmetic `1+96*4096+1048576 = 1,441,793` (now `1+96*65536+4194304 = 10,485,761`). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
Follow-up on the parts we said we would sort into "taken" and "not taken". The change is in #541 (engine 1.8.0, 27cd77b). Taken. The shape you proposed: walk only the live cohorts instead of every cohort row. DiskResidentIdTable now keeps an ordered held set of the resident rows that hold an opening or live units (held_rows()), refreshed after each writer, and the walks that need only those rows use it, so pending-only cohorts are no longer visited. The commit carries a credit trailer for you. Taken, found while measuring it. A close event on N owners no longer seeks the scratch index once per owner for every bracket family: BracketRoster::remove_selected checks the in-memory incarnation counts first. On the dense shape (the position goes flat by a stop fill, not by a script close_all) a test counts find_key calls: 8,385 and 33,153 before, 257 and 513 after (two sizes; counts printed by the test, not seconds). Not done.
Measured result of the whole change. Release build of each tree (engine main and #541, same generated C++), public ETH-USDT 1m feed, first N bars, default inputs; jailed = sealed 1 GiB memfd, unjailed = default profile; 4 repetitions per cell up to 30,000 bars, 2 at 60,000 and 100,000; 600 s cap per run, so a cap hit means "more than 600 s", never a time. Wall seconds are medians of the completed runs; 8 runs shared one 16-vCPU box, so absolute seconds carry load (repetitions agree within 1-12 percent).
Not measured: where the dense shape spends its time past about 4,000 bars, any wall above 600 s, unjailed RSS growth of the dense shape beyond 4,000 bars. Also in #541, a behaviour change you should know about: the jailed profile gets a 64-byte cap on a single id and a larger scratch share (a fixed resident floor of about +51 MiB per jailed run, measured +52 MB peak RSS); the PR lists it. Thank you for the proposal and the reproducer. |
Refs #452.
What
PineExecutionAdapterkeeps a cohort incohorts_by_id_/cohort_order_for every entry id the run has used, and the core keeps aCohortRosterfor each of their handles; neither is ever pruned, and neither should be (origin history and the state hash read them). Several walks that only readopenedandlive_units_by_originwent over all of them: the open-entry count the scheduler publishes every bar (source_entry_slot_count), the two walks inerase_retired_rows, the bar-open roster pass inrelease_closed_cohort_origins, and some twenty more at fills and placements. A script that gives each position an id of its own therefore paid the square of its trade count. This change addsheld_cohort_ids_, the ids whose cohort holds an opening or live units, kept incohort_order_order (which is handle order) asopenedandlive_units_by_originchange, and points those walks at it.release_closed_cohort_originskeeps a memo of the rosters that hold members, fed by the core's append-onlycohort_receipts()(only an applied add puts a member in a roster), and its four loops read those rosters only. Both are derived, not state:held_cohort_ids_is waived inbroker_state_hash_waivers.txtlike the other indexes, and the roster memo lives inretired_row_scratch_, whose waiver now says so. In every non-Release read,held_cohort_ids()asserts the index equals the cohorts a full walk finds, in the same order, andsource_entry_slot_countand the roster pass assert their totals against the walks they replace. Trade lists and equity are unchanged.Not covered: a fill that reverses the position still collects every cohort id the run has used and calls
cancel_exit_orders_for_full_closefor each (on_applied, theflipped_positionbranch). For an id that matches nothing, that call still sweepsbracket_families_, so narrowing the set is not obviously equivalent; I left it for you to judge. On the synthetic below, the reversing variant goes from 219.6 s to 23.6 s, against about 1.4 s with fixed ids.The anchor moves are mechanical: every
pine_adapter.cpp/pine_adapter.hpppath:lineanchor in docs and test comments was remapped to the line its symbol moved to (check_doc_anchors.py: 1742/1742).scripts/corpus_parity_mutation_battery.tsvis left alone: its lines already name an older tree at base.Evidence
New row
uniqueidsintests/test_adapter_live_state_scaling.cpp: an opening every 20 bars under an id of its own, closed 10 bars later, no recording. Failing before, this TU against base0212cabe(macOS 15.8 arm64, Apple M1 Pro, Apple clang 17, Release):After:
python3 scripts/ci_verify.py release --build-dir build-ci-release --jobs 8(full population, no label excluded),build-ci-release/ci-summary.json:ci_verify.py kernel: not run. Its configure stops atCould NOT find CURL: Found unsuitable version "8.7.1", but required is at least "8.14.1"(the macOS SDK's libcurl, for the native runner). This change only compiles withPINEFORGE_BUILD_SOURCE_LAYER, which that profile turns off.ctest, Debug build with the new asserts on, every target,-j8 -E '^test_ci_verify$':That row expects a
RecursionErrorthat Homebrew Python 3.14.6'sjsonno longer raises; it fails the same way on base0212cabeand passes under Python 3.14.4, whichci_verifyused../scripts/check_corpus_parity.sh --subset:./scripts/check_corpus_parity.sh(full sweep):python3 scripts/ci_preflight.py: 48 passed, 4 failed, 1 skipped.actionlint-version,shellcheck-versionandworkflow-lintfail because neither tool is installed here;run-json-harness-testshas 9 failures from Python 3.14's argparseprog(usage: python3 -m pytest ...), identical on base.The script from #452 and three variants (fixed ids; reversing with
strategy.entryalone instead ofstrategy.close_all()first), Binance spot BTCUSDT 5m, 2020-01-01 to 2026-09-07, 702,505 bars, 13,500 trades each, onerun_backtest_fullper run, strategy built-O2against each tree's Releaselibpineforge.a. Trades and equity hashed field by field; every run on both trees printedtrades=13500 net=-286.760100 trades_sha=d6a9d963a7427d46 equity_sha=658408382d422c20 equity_len=702505.0212cabeclose_allfirst (#452)close_allfirst (median of 10)Out of scope: with recording on,
hash_statestill walks every cohort, and each id used keeps about 5 KB (about 271 MB RSS at 51k ids).🤖 Generated with Claude Code