Conversation
lvkale
force-pushed
the
objid-redesign
branch
from
September 30, 2026 03:50
1430736 to
226a05e
Compare
Contributor
Author
|
Rebasing objid-redesign (and the stacked objid-pr1a-layout / objid-pr1b-allocator) onto reviewed-with-reconverse d17228d now, to pick up the checkpoint-restart fix (#4019): on Anvil the 1a+1b branch passed everything except restarts with >1 process and >1 PE per process, which hang in exactly that pre-fix path. Force-pushes follow; no content changes beyond the rebase. |
…rray's index is not packed into ids An array with no index compressor mints element ids from a per-PE counter in the element field (16 bits by default). Nothing checked the counter: past 65,535 elements on one PE it carried into the home field, so ids silently collided with another PE's and every home lookup for them went to the wrong PE. getNewObjectID now aborts at the limit, naming the location manager and the remedies (insert from more PEs, setBounds, fewer collection bits). The message is kept under 255 characters because reconverse's CmiAbort formats into a 256-byte buffer. Whether an array gets a compressor is easy to get wrong by accident: the sized CkArrayOptions constructors set bounds, but setNumInitial/setEnd do not, and bounds that need more than CMK_OBJID_ELEMENT_BITS are refused silently. The CkLocMgr constructor now prints one note on PE 0 in either case, with the bit count it needed or the setBounds remedy. A public FixedArrayIndexCompressor::bitsNeeded(bounds) reports the width; make() uses it too. Diagnostics only; no change to ids or delivery. Part of the object id work tracked in #3994 (PR 0 of doc/objid64-design.md on the objid-redesign branch). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… index is not packed into ids Every migrating test in the tree uses a small bounded array, whose index is compressed into the element id, so the other id scheme (per-PE counter plus an index<->id map in CkLocMgr) was never exercised together with migration. This test creates a 2D array with no bounds and no initial size, inserts every element dynamically from PE 0, and runs a message ring in which each element migrates to the next PE every five steps; it ends by quiescence. Default 4x3 elements, count 40; arguments nX nY count. Registered in the regular test list (anytime_migration is in FTDIRS and only runs under syncfttest). Part of the object id work tracked in #3994. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two documents for the object id redesign discussed on charm #3994: - doc/objid64-design.md: the detailed design. 64-bit id with a 12-bit collection field (build-time) and a 49-bit payload; bounded arrays pack their index into the whole payload; unbounded arrays carry a hash key of the index (width fixed per run from the process count and an expand factor) plus a unique number handed out in tranches, so the home of an element is computable from the id or the index alike and no PE number is stored in the id. Ids are stable across restart and shrink/expand; directories are rebuilt. Delivery by id never needs the index off the source PE. A per-process directory and cache follow as a later PR. Includes the PR sequence, test matrix, and open questions for Aditya. - doc/objid64-analysis.md: the analysis behind it: the id -> home -> index -> home -> location chain as the code runs it today, the seven defects that follow from having two homes, and every scheme considered with why it was kept or dropped, including the 128-bit option that was rejected. Merging the implementation is deferred until the reviewed line is stable with users and the test suite is broader; the diagnostics and the hashed-kind migration test (PR 0 in the design) can land first. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ranch The object id redesign (#3994, design in doc/objid64-design.md) is developed as a stack of pull requests whose base is objid-redesign, merged to the reviewed line once at the end. The reconverse workflows trigger only on the reviewed line, so those pull requests and pushes to the branch would get no CI. Add the branch to the push and pull_request triggers of both workflows. Drop this commit, or leave it (harmless), at the final merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ge within a run) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ag bits Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…kCallback variant as a later item Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hared-table ordering, LB global update epoch, restart) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Kale's point (2026-09-30): a message for an element that lives on another PE of the same process must never go to the index home for its location. Stated as five guarantees (resident elements resolve locally; departed elements leave a forwarding entry; one request per process; the home answers from the shard on any rank; same-PE delivery unchanged) with what each needs from the shared idx -> id and id -> location shards. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… re-send, lock-free minting, allocator clamp, restart without waiting) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…esign 2 and 4.3 note the sender-repair rule applies to both kinds Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed objid-pr* branches Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…es, sweep on a cap, what eviction costs) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design documents for the 64-bit object id redesign discussed in #3994, on the
objid-redesignbranch where the implementation will be developed. Opened as a draft: merging the implementation is deferred until the reviewed-with-reconverse line is stable with users and the test suite is broader. The documents are in the repository so the work does not depend on anyone's laptop.doc/objid64-design.md (the design):
CMK_OBJID_COLLECTION_BITS; AMPI may need 21 or 24) | payload 49.CMK_OBJID_HOME_BITSgoes away: no PE number is stored in the id.handleUnknownByID(e417584) is folded in.doc/objid64-analysis.md (why): the id -> home -> index -> home -> location chain as the code runs it today, the defects that follow from having two homes (D1-D7), and each scheme considered with why it was kept or dropped, including the rejected 128-bit option.
Aditya has confirmed the design subsumes the location-manager work on his
rate-aware-gpu-lbbranch; ideas and code from there will be cherry-picked as the implementation proceeds. Kale and Eric Bohm for design review.PR 0 from the design (overflow abort in
getNewObjectID, the no-compressor note, and a migrating test for a non-compressible index) changes no behaviour and can land on the reviewed line ahead of everything else.🤖 Generated with Claude Code