Repository navigation
DO NOT MERGE: lxr improvements for julia - #1564
Draft
oscardssmith wants to merge 61 commits into
Draft
oscardssmith wants to merge 61 commits into
oscardssmith wants to merge 61 commits into
Conversation
Everything needed to run LXR as Julia's GC, as one commit to be split into reviewable pieces. Based on wenyuzhao/mmtk-core lxr-x/simplified. Grouped by where each piece should eventually go: Independent of LXR, targets mmtk-core master: - ConcurrentImmix schedules its root-nodes work with `add_deferred` and enables the Concurrent bucket for the pause, so packets added during the pause become available when the bucket flips. Bugs in LXR itself: - LargeObjectSpace released the object reference's address rather than the super page of `to_object_start()`. Correct only for VMs that set UNIFIED_OBJECT_REFERENCE_ADDRESS; for anyone else it hands the page resource an address it never allocated. Three sites. - Line-occupancy marks were wrong in three ways: the last line went unmarked when an object did not end on a boundary, the line holding the header was never marked when the reference address sits past the allocation start, and RC_STRADDLE_LINES was per line, so two objects sharing a line raced to clear each other's mark. It is now per granule, and marking and unmarking share one function so they cannot cover different sets of lines. - `rc_sweep_nursery` trusted the in-place-promotion flag and asserted `rc_dead()` only under debug_assert, so release builds freed blocks with live objects in them. It now checks the counts. - Root objects reported as nodes were incremented but not promoted, leaving a nursery root's referents at zero; and nodes the plan does not reference count were dropped instead of seeding the trace. - Slots outside the heap have no field unlog bit, and computing a metadata address from one reads unmapped memory. Immortal and VM space now arm the field unlog bits when the plan uses a field-granularity barrier. - `Slot::is_derived` lets the coalescing barrier decline to record a slot whose referent can only be recovered from state captured when the slot was made. Re-reading such a slot at the next pause yields an interior address that is not an object. Related to mmtk#1038. Not for upstream: - An object-granularity barrier variant behind `lxr_object_barrier`, for VMs that cannot name the written field. Incomplete: epoch re-arming is not implemented, so it is not correct to run. Julia does not need it. - Bring-up diagnostics: per-pause statistics, a reference-count event ledger, live-set tracking, and the retain-nursery/retain-mature switches, all behind MMTK_LXR_* environment variables. The retain switches reach into policy/immix/block.rs, which is the wrong place for them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`CollectNodeRoots` only ever traced its root set inside a stop-the-world pause. Roots reported as *slots* reach the concurrent workers through `ProcessIncs`, but a binding that reports roots as objects -- Julia does, via `create_process_pinning_roots_work` -- arrives here instead, so an `InitialMark` pause seeded no concurrent work at all. With nothing seeded, `concurrent_marking_packets_drained` is trivially true, so the trigger in `should_do_cycle_collection` fires immediately and the whole transitive closure lands in the following `FinalMark` pause. LXR then behaves as a stop-the-world marker, with a pause proportional to the live heap regardless of how much time the mutator had in between. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ImmortalSpace::prepare` and `VMSpace::prepare` bzero the mark bits of every region they own. For a plan that marks in a single pause that is right, but LXR marks between `InitialMark` and `FinalMark`, and `Plan::prepare` ran for both. Clearing at the closing pause discarded everything concurrent marking had established, so the pause re-traced the whole graph -- for Julia, whose sysimage lives in the VM space, about 1.77M objects re-marked in every `FinalMark`. `CommonPlan::prepare_ext` and `BasePlan::prepare_ext` take the decision as a parameter, and LXR resets only in the pauses that begin a mark cycle (`Full` and `InitialMark`). `prepare` keeps its old meaning, so no other plan changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two costs dominated the barrier for a VM whose fast path cannot always name the field written. `slot_has_unlog_bit` walks the mmapper's two-level chunk table, which is more expensive than everything else the barrier does, and callers ask it for field after field of one object -- all in the same chunk, since side metadata is mapped with the chunk its data belongs to. The last chunk that answered yes is remembered per mutator and reused. Only positive answers are cached: a chunk that is unmapped now may be mapped later, and skipping a slot that does have a bit loses an increment permanently, while metadata is never unmapped, so a cached yes cannot go stale. `object_probable_write_slow` snapshots every field of the object, and nothing stopped it running again on the next store to the same object. It now logs the object as well as its fields, which is what a fast path that can only test the per-object bit (Julia's `mmtk_gc_wb_fast`, for stores whose field it cannot name) keys on, so the walk happens once per object per epoch instead of once per store. Field bits re-arm lazily as each increment is processed; there is no per-object equivalent, because the increment buffer holds slots and a slot does not identify its owner, so the logged objects are remembered and re-armed by `flush` at the end of the epoch. Leaving one logged past its epoch would lose every later store to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Plan::release` ended every pause by re-arming the log bit of every object in the heap: `ImmixSpace::set_side_log_bits` walks every chunk single-threaded (its own `warn!` says as much), and `CommonPlan::set_side_log_bits` enumerates every LOS object one atomic store at a time. That makes a pause cost O(heap) rather than O(work done in the pause) -- 1.3ms of the 4.4ms median `RefCount` pause on `tree_mutable`, serially, inside a single `Release` packet, and it grows with the heap. Only bits a barrier actually cleared need re-arming. The field barrier already records the objects it logged and re-arms exactly those on flush, and mutators are flushed during every pause, so the bits are armed before mutators resume -- the only property the old location provided. The object barrier now does the same, recording pages rather than objects. The log bit is side metadata, so re-arming a whole page costs one `bset_metadata` of that page's metadata range, the same work as re-arming a single object on it, and it collapses the many objects a mutator typically logs on one page into a single entry. Over-arming is harmless: an armed bit only means the barrier fires once more for that object, which is the state every object starts an epoch in anyway. This is the epoch re-arming that `lxr_object_barrier` was missing, and without which it was not correct to run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`record_rc_event` sits on every processed increment and decrement, and `is_known_live` is consulted for every object a decrement takes to zero. The first inlined its whole body including the formatting and map lookups; the second took a global `RwLock` even when the verifier had never recorded a live set, so every GC worker contended one cache line for an answer that was always `false`. The recording is now out of line and `#[cold]`, and an atomic flag says whether the live set has ever been populated, so the disabled case is a predictable load and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`MMTK_LXR_STATS` now times each work packet and reports the types that accounted for the most time since the last report. `work_packet_stats` already measures this but only reports at harness end, which cannot answer "what is this one pause spending its time on" -- the question that matters while reducing a pause. The table is reset when the world stops, so it describes one pause; without that it spanned from the previous `resume_mutators` and absorbed all the concurrent work executed during the intervening mutator window, which is how entries came to report far more time than the pause they appeared under. Packet timing sums CPU across all workers, so it still cannot say where a pause's *wall* time goes: a 1ms pause can contain 30ms of packet time. `stage_timeline` records ordered marks against a single clock instead, so the gap between consecutive marks is elapsed time on the critical path -- including the park/wake handshake between work-bucket stages, which no packet timer sees. Marks are placed where every worker is provably parked (so the gap is a whole stage) and around the phases of LXR's `Release`. LXR's release also reports where the pause's own phases went (weak processing, `vm_release`, the common plan) and how many concurrent tracing packets are still outstanding. Zero packets at the end of an `InitialMark` means nothing was handed to the concurrent workers and the closure will fall into the next `FinalMark`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Scanning::scan_chunk_count` reports how many pieces an object's reference fields can be visited in, and `Scanning::scan_object_chunks` visits one range of them. A VM that does not implement the pair keeps the existing behaviour: the default count is `None`, and MMTk scans whole objects. This exists so that one object need not pin one worker. An object whose field count grows without bound -- a 100M-element array of references -- takes hundreds of milliseconds to walk, and no amount of buffer-level splitting reaches it, because the cost is the walk itself rather than the work it discovers. The count is deliberately not a field count: a chunk is whatever unit the VM can start and stop at, so an array of multi-reference structs can report elements and still be split. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`scan_nursery_object` walked every field of a promoted object inline, so one worker did the whole object while the rest of the pause's workers had nothing to do. For Julia's 100M-element array of `ZonedDateTime` that walk was a 311ms stop-the-world pause on its own, and buffer-level splitting could not touch it: the elements nearly all refer to the same already-counted object, so the walk takes the increment branch and spills almost nothing into the increment buffers that the existing packet splitting works on. An object the VM offers in chunks (`Scanning::scan_chunk_count`) is now handed to the other workers as one increment packet per 8192 chunks. The per-field work moved into `count_promoted_field` so the inline and packet paths cannot diverge, and the increments a chunk discovers are flushed by the same path as any other. Measured on GCBenchmarks' TimeZones, 16 workers: worst pause 311ms -> 98ms, total GC time 486ms -> 305ms. The pause that remains is the same object: 1.29s of worker CPU across 12324 packets, so it is now bounded by how many workers there are rather than by the array's length. Getting it below a millisecond needs the counting to move out of the pause altogether, which is a change to when increments may be applied, not to how they are split. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The packet timings say which work packet types a pause spent its time in, but not what that time bought. These are the denominators: increments handed to `ProcessIncs`, objects it promoted, and the calls the slot-less write barrier had to answer by walking a whole object because the caller could not name the field written. Time divided by increments is what distinguishes a pause that is expensive because there are many increments from one where each is slow, and fields-per-call is the amplification factor of the barrier path that Julia's codegen falls back to. Each is maintained only under `MMTK_LXR_STATS`, and accumulated per packet or per call rather than per increment or per field: a global atomic on those paths is millions of contended updates inside the pause, enough to inflate the pause it is supposed to be measuring by 8x. The barrier's counter is one atomic per call, `ProcessIncs` publishes its tallies once when the packet finishes, and the plan reports and clears them at the end of the pause. `select_collection_kind` also reports the inputs to its own decision, since a pause kind that looks wrong is almost always a decision taken on numbers that look nothing like what the heap is doing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The promotion trace was a chain, not a tree. `add_new_slot` spills a packet every 1024 slots, and a packet consuming 1024 slots promotes about 512 objects whose fields are about 1024 new slots, so each packet produced almost exactly one successor. Measured on `tree_mutable`: 2.08M increments in 1984 packets of ~1048 slots each, formed as 13 chains -- one per root packet -- about 152 packets long. Parallelism was capped by the number of root packets rather than the number of workers, and measured about 3x (the root buffers are uneven); `ProcessIncs` CPU over pause wall time stayed at ~3x whether 4, 16 or 64 workers were available. `ProcessIncs` now hands half of each generation to another worker once the generation is larger than `MMTK_LXR_SPLIT_MIN` slots, which turns the chain into a binary tree whose depth is logarithmic in the generation size rather than linear in it. The dead `ACTIVE_PACKET_SPLIT` switch it replaces only split below a depth threshold, which is the wrong axis: depth is not what bounds the trace, width is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every work-bucket boundary woke every parked worker, and several of the buckets an LXR pause opens hold exactly one packet (`ScheduleCollection`, `StopMutators`, `ScanMutatorRoots`, `FastRCPrepare`, `Release`). All but one worker immediately found nothing and re-parked, paying for the monitor lock twice more each. The bucket-opening path now reports how many packets it made available and the last parked worker wakes that many, which costs one `notify_one` per worker that actually has something to do. Under-waking cannot strand work: a worker that adds to an already-open bucket notifies through `WorkBucket::add`, so a packet appearing later gets its own wake-up. `MMTK_WAKE_ALL=1` restores the old behaviour for comparison. `notify_work_available` also took the monitor lock unconditionally, once per packet added, to notify condition variables that usually have no waiter at all -- every worker is busy while a GC is running. It now tracks how many workers are blocked and returns without locking when that is zero; the ordering argument for why a zero reading cannot lose a wake-up is on the function. Broadcasts to `active_worker_number_changed` are likewise gated on a worker actually waiting to be reactivated. The exhaustive victim scan in `poll_slow` looks like the obvious remaining O(workers^2) waste, and is left alone deliberately: probing a few random victims instead left the minimum pause unchanged -- so the scan is not on an empty pause's critical path -- while median pause and wall time got much worse (32 workers: `strings` median 6.9ms -> 21.7ms, `tree_mutable` wall 23.0s -> 34.0s), improving monotonically as the probe count rose back to exhaustive. `WorkBucket::poll` is a batch steal, so an opening bucket's packets sit in a few workers' local deques; finding them is what keeps the phase parallel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A concurrent phase whose work simply runs out has nobody to announce it. LXR's concurrent decrements and sweeping end that way -- unlike ConcurrentImmix, whose concurrent marking always finishes in a `FinalMark` pause and so reports through `resume_mutators`. Until now the collector stayed in `InConcurrentGC` and looked permanently mid-GC to everything that waits for it to be quiescent, `set_disabled` in particular. The last parked worker finding no goal to respond to is exactly that moment: a pause would have arrived here as a request. It now returns the status word to `NotInGC` and calls `Collection::concurrent_work_finished`, which a binding can use to release a thread that is waiting for the collector to go quiet. The transition reports whether it happened, so a mutator that requested a pause in the meantime is not overwritten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A plan that reclaims lazily has freed almost nothing by the time `on_gc_end` runs: LXR hands its nursery and mature block sweeping to the concurrent phase that follows the pause, and the page resource is not flushed until that phase drains. A trigger policy that sizes the next heap target from the reserved pages it sees at `on_gc_end` therefore concludes the collection freed nothing, and grows the heap without bound -- measured on Julia's `tree_mutable`, 1.6GB to 6.4GB peak RSS. `GCTriggerPolicy::on_lazy_reclaim_finished` is called once that phase has drained and flushed, which is the first moment the freed pages can be seen through `get_reserved_pages`. It takes the plan rather than the `MMTK` instance because the plans that reclaim lazily reach it from their own bookkeeping, which has no `&'static MMTK` to hand; a policy needing per-cycle facts from the instance should latch them in `on_gc_start`. A plan that reclaims everything before `on_gc_end` never calls it, so a policy must still leave a usable target behind at `on_gc_end`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The decision about the next collection is taken when the previous cycle's deferred sweeping drains, and it reads how much that cycle freed. Both halves of that were wrong. `num_clean_blocks_released_lazy` is a monotonic total, but `LXR::prepare` zeroed it at the start of every pause, so a decision taken just after a pause compared this cycle's reserved page count against a counter that had just been cleared. The heap looked full and the collection looked like it had freed nothing, which hinted an emergency and forced a whole-heap stop-the-world trace -- that accounted for every emergency pause on `tree_mutable`. The counter is now left alone and `gc_pause_end` snapshots it, and the LOS equivalent, alongside `HEAP_AFTER_GC`, so all three describe the same instant and the reclamation since then is a difference. `end_of_lazy` also fired for any wave of jobs that happened to reach zero. A wave is everything that became owed between two consecutive swaps, and `swap` runs once per pause, so a wave belongs to exactly one cycle -- but the wave still accumulating can hit zero transiently, and an old wave can drain late. Either would pair the newest `HEAP_AFTER_GC` with reclamation that has not happened yet. Waves now carry the cycle they belong to, only the current cycle's wave takes the decision, and it takes it once. A pause that deferred nothing decides immediately, since no wave will arrive to do it and `wait_for_decide_cycle_collection` would otherwise wait for nobody. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`select_collection_kind` treated the low-headroom hint exactly like a genuine allocation failure and answered both with a full stop-the-world trace. Those full traces measured about 300ms each on `tree_mutable` and were 70% of all pause time, against about 2.4ms for an `InitialMark` and 17ms for the `FinalMark` that closes it. Low headroom is a reason to *trace*, not a reason to stop the world: tracing concurrently is what this plan exists for, and the hint says the heap needs its cycles found, not that memory has run out. It also fires readily, because the headroom test compares the budget against reserved pages less the whole blocks deferred sweeping returned, while LXR reclaims mostly by recycling lines inside blocks that stay reserved -- so reserved pages sit near the budget whether or not memory is actually short. A hint that fires that easily cannot carry the weight of a whole-heap stop, and does not need to: a real exhaustion still arrives as `emergency`, and a user-requested collection still gets its full pause. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`increase_inc_buffer_size` was a load, an add and a store, and every mutator's barrier flush lands there. A read-modify-write that is not atomic loses almost every update under that much contention: measured on `tree_mutable`, the counter read back between 0 and 10918 for pauses whose `ProcessIncs` packets handled millions of increments. Any trigger built on `inc_buffer_size` was therefore inert. It is a `fetch_add` now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A pause's cost is dominated by `RCProcessIncs` promoting the young objects that survived -- 12.9ms of a 14.2ms `RefCount` pause and 5.5ms of a 7.9ms `FinalMark` on `tree_mutable`, with everything else under a millisecond. The two bounds meant to cap that were both ineffective. `MAX_SURVIVAL_MB` predicted survival from `SurvivalRatioPredictor::ratio`, which counts only promotions that *copied* the object. With evacuation disabled -- permanently, for the Julia binding -- that ratio is pinned at zero, so the prediction was zero against a 128MB limit and the bound never once fired, while about 887k objects and 27MB were promoted per pause. The predictor now also tracks the fraction of young allocation that survived at all, copied or promoted in place, and the bound uses that: a pause pays for every promotion, because `ProcessIncs` scans each promoted object's fields and generates further increments from them whether or not the object moved. `ratio` keeps its meaning for sizing a to-space, where being zero without evacuation is correct. `INC_BUFFER_LIMIT` has always been declared here and never read, so nothing bounded how much mutation could accumulate between pauses: heap occupancy is not a bound on that, and a mutation-heavy program reaches the heap target with an unbounded increment buffer queued. It is now consulted, off by default, via `MMTK_LXR_INC_BUFFER_LIMIT`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sweeping a nursery block reads the block's reference count table to decide whether it is dead, reusable, or promoted in place. That is O(nursery bytes), it ran single-threaded in the `Release` packet, and it measured 7-8ms of a 27ms `RefCount` pause on `tree_mutable` -- second only to `RCProcessIncs`. None of it has to happen while mutators are stopped. A block being swept is not yet on any free list, so no mutator can allocate into it either way, and the concurrent phase that runs it (`RCLazySweepNurseryBlocks`) already reports what it frees through `num_clean_blocks_released_lazy`, which is what the next pause's sizing decision reads. The only cost of deferring is that the pages come back a mutator window later. `MMTK_LXR_STW_SWEEP_BLOCKS` caps how many blocks a non-`Full` pause sweeps itself, defaulting to none. A `Full` pause still sweeps everything: it is already a whole-heap stop, and it must not leave work owed to a concurrent phase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The forwarding bits are read and written by object forwarding, which only happens during evacuation. A build with both nursery and mature evacuation compiled out never forwards anything, so it has no reason to reject a binding that keeps its forwarding bits on the side. Julia's object model does exactly that, and its non-moving configuration could not start up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oscardssmith
marked this pull request as draft
August 24, 2026 21:56
# Conflicts: # src/plan/lxr/gc_work/mature_sweeping.rs # src/plan/lxr/gc_work/tracing.rs # src/policy/immix/immixspace.rs # src/util/rc.rs
# Conflicts: # src/plan/lxr/gc_work/tracing.rs # src/plan/lxr/global.rs # src/scheduler/scheduler.rs
…radual, upstream-audited rewrite Ports mmtk-core-lxr-followup3's fix from scratch as a series of small, individually-verified commits on top of current upstream master (see the lxr-julia-obj-ref-gradual branch on the gradual worktree for the full history), rather than as one large change. This caught two things the prior version missed: - object_is_in_straddle_line() had the same missing-position-check bug as object_is_in_straddle_line_no_rc_check(), just reached through count(o) != 0 instead of a raw metadata read. Fixed by delegating to the already-fixed function instead of re-deriving the check. - Current upstream master carries two straddle-line pre-filters (in ImmixSpace::trace_object_without_moving_rc and SweepDeadCycles::process_block) that assume a mark can only ever sit at a line's own start -- true for UNIFIED_OBJECT_REFERENCE_ADDRESS VMs, not for Julia, where a header mark can land in a line's last few bytes. Left unguarded, both would have silently misidentified a live header mark as a real, dead object during a sweep. Both are now gated on UNIFIED_OBJECT_REFERENCE_ADDRESS, keeping the exact same fast path for unified VMs and falling back to the exact check for Julia. The core design is unchanged from before: mark_straddle_object_with_size marks an object's header line (when the reference address trails the allocation start) and tail line (general LXR fix, not Julia-specific -- also needed by unified VMs, see the gradual branch's second commit) with synthetic RC entries, tagged with independently-addressable straddle bits so a real object sharing that line is never misread as a mark. Also restores the historical bring-up narrative this file carries (the inc_buffer_size fetch_add fix, the MAX_REF_COUNT-encoding alternative considered for the sweep crash) that the gradual rewrite deliberately leaves out, per JULIA_LXR_UPSTREAM.md's rule to keep that out of the destination repo. Verified: OpenJDK/fastdebug DaCapo chopin (sunflow x2, avrora x2) via the JULIA_LXR_UPSTREAM.md recipe, all passing, exercising the unified fast path with the two restored pre-filters active. Julia build (WITH_THIRD_PARTY_GC=mmtk MMTK_PLAN=LXR) passes; a GC-stress script mixing tiny/sub-line/exactly-one-line/multi-line/large allocations with a cohort kept alive across 40 rounds of GC.gc(true) (to force block reuse while still live) produces an identical checksum and intact objects across 3 runs, exercising the non-unified header-mark path this change adds.
This was referenced Aug 31, 2026
# Conflicts: # Cargo.toml # src/plan/lxr/gc_work/mod.rs # src/plan/lxr/gc_work/rc.rs # src/plan/lxr/gc_work/tracing.rs # src/plan/lxr/global.rs # src/scheduler/work_bucket.rs
RC_STRADDLE_LINES was written from the object start but read from the object reference address. The two coincide for a binding with a unified object reference address, so the mismatch was invisible on OpenJDK. For a binding whose reference address trails the start (Julia's by 8 or 16 bytes in the immix space), an object beginning in the last few bytes of a line has its reference address in the next line. `Line(start).next()` is then the line holding the object's own count, and marking it does two things to a live object: `set_line_relaxed` overwrites its reference count with 1, because the RC granule is a word and the line start shares that granule; and every straddle query reports it as a synthetic line mark, so tracing skips it and SweepDeadCycles reclaims it while it is still reachable. Derive `start_line` from the reference address in both the mark and the unmark so the write agrees with the read. The loop bound has to become `<` rather than `!=` as a result. `start_line` can now exceed `end_line`: with 256-byte lines, an object at start ≡ 240 (mod 256) with a 16-byte offset and size 257 has its reference address on the next line's first byte and its end only 241 bytes into that same line, so there is no line left to mark. `!=` would walk forward forever writing marks across the heap. The old bound could not hit this because Line(start).next() <= Line(start+size) whenever size > Line::BYTES. Julia builds and runs again with MMTK_PLAN=LXR: make, sysimage.mk sysimg-release, and pkgimage.mk release all pass with no GC crash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
qinsoon
added a commit
to qinsoon/mmtk-core
that referenced
this pull request
Sep 7, 2026
This PR implements `create_process_pinning_roots_work` for `LXRRootsWorkFactory`, and adds a bucket before the normal `ProcessIncs` to handle pinning roots. Changes are originated from mmtk#1564. Mostly done by Claude.
This reverts commit b63d197.
# Conflicts: # src/scheduler/scheduler.rs # src/util/rc.rs
…sign Adopt the upstream design from 2eddba8 in place of this branch's equivalents: - Drop the `LXRSpace` enum and `LXR::space_of`. Tracing and reference counting now dispatch on `Space::in_space` directly, as master does. The only remaining use of a space classification was in the malformed-increment diagnostics, which get a local `space_name` helper instead. - `LXR::prepare` inlines the per-space `prepare` calls under `starts_mark_cycle` rather than going through `CommonPlan::prepare_ext`. `prepare_ext` (on both `BasePlan` and `CommonPlan`) had no other caller and is removed, so src/plan/global.rs now matches master exactly. - `LXR::mark`/`is_marked` take master's chained form, including its `debug_assert!(o.is_live())` for common-plan spaces. Behaviour is unchanged for the configurations Julia uses: master guards code/ro-space prepare on `starts_mark_cycle` where prepare_ext did not, and `prepare_nonmoving_space` early-returns for LXR either way.
Align with 4011008 where this branch had kept its own equivalents: - `ProcessIncs::process_root_nodes` uses master's `LXR::is_rc_object` / `los().in_space()` classification instead of open-coding it. The branch's `debug_assert!` that a root node has not already been forwarded is kept on top, as master has no equivalent check. - `LXR::is_rc_object` moves to master's position in the impl block (just after `los()`) with master's doc comment. The rest of mmtk#1572 (RCProcessIncsNonMoving, CollectNodeRoots/CollectSlotRoots, PinningRootsTrace, LXRStopTheWorldProcessNodes) was already present and now matches master's code; only added doc comments differ.
Instrumentation removed (all of it added during this branch's LXR bring-up and never intended for upstream): - `scheduler::work::packet_timing` and `scheduler::work::stage_timeline` diagnostic modules, their `scheduler` re-exports, the per-packet timing wrapper in `GCWork::do_work`, and every `mark`/`start` call site. - The `MMTK_LXR_STATS` reporting in `LXR::new`, `release`, `decide_next_gc_may_perform_cycle_collection` and `select_collection_kind`. - The bring-up verifier: `LIVE_SET`/`set_live_set`/`is_known_live`, the `DEC_SITES` reference-counting ledger (`record_rc_event`, `dump_rc_events`, `report_dec_of_live_object`), `field_is_logged`, `object_is_unlogged`, `object_is_plausible`, `check_incs` (`MMTK_LXR_CHECK_INCS`) and `ProcessIncs::report_malformed_inc`. - Counters with no consumer left: `OUT_OF_HEAP_SLOTS`, `SWEEP_ZEROED`, `SWEEP_KEPT_MARKED`, `sweep_dead_cycle_counts`, `RELEASE_STARTED`, `INCS_AFTER_RELEASE`, `OBJ_WRITE_CALLS`, `OBJ_WRITE_FIELDS`, `INCS_PROCESSED`, `OBJS_PROMOTED`, and `ProcessIncs`'s local `incs_count`/`promoted_count`. - The `lxr_promote`, `lxr_inc_pushed`, `lxr_inc_processed` and `lxr_slot_skipped` USDT tracepoints, the `SkipReason` enum they carried, and their section of tools/tracing/README.md (that file now matches master). - Bring-up switches that only leak to isolate a suspect: `MMTK_LXR_NO_FULL` (`no_full_pauses`), `MMTK_LXR_RETAIN_NURSERY` (`retain_nursery`) and `MMTK_LXR_RETAIN_MATURE` (`retain_mature`), plus `stats()`. - `LXR::object_occupies_free_line` (dead) and `LXR::snapshot_curr_roots`. Kept: `GC_COUNT`, which is not a diagnostic -- it is the epoch that identifies a wave of deferred lazy-sweeping jobs -- now documented as such. Also kept are the four env-tunable knobs that change collection policy rather than report on it: `inc_buffer_limit`, `active_packet_split`, `max_stw_sweep_nursery_blocks` and `max_survival_mb`. Separately, every comment-only difference from master is reverted, so the diff against master is now code-only (91 hunks, none comment-only). Findings that those comments recorded are kept out of tree in LXR_FINDINGS.md. Divergence from master: 29 files/+2123 -> 24 files/+1126.
The comment-only revert in ba7a990 deleted this line by mistake: the classifier that selected comment-only hunks treated any line beginning with `*` as a block-comment continuation, which also matched the Rust deref `*VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.as_spec()`. Without it the per-object log bit's side metadata is never mapped, and Julia's immortal post-alloc fast path (`mmtk_immortal_post_alloc_fast`, which writes that bit directly) segfaults on the first permanent allocation in `jl_gc_permobj` during `ijl_init_` -- i.e. the sysimage build dies immediately. The rationale that used to sit here as a comment is in LXR_FINDINGS.md (item 7).
Restores 91bca96, whose functional half was missing on this branch: `GCTrigger::poll_from_last_parked_worker` is added verbatim from master (including the `record_pause_requested_time` call a later commit added to it). The branch had put its own logic at the same point in `respond_to_requests`: when a concurrent phase drains with no goal pending, it leaves `InConcurrentGC` and calls `VMCollection::concurrent_work_finished()` so a thread parked in `mmtk_wait_for_new_gc_epoch` is woken. That is not an alternative to the poll but the other half of the same decision, so the two are now ordered: 1. `poll_next_goal()` -- a mutator-requested goal wins. 2. otherwise `poll_from_last_parked_worker()` -- under heap pressure, request the concurrent GC's closing pause here rather than waiting for a mutator to allocate into it. 3. otherwise the concurrent GC really is over, so run the branch's finished/notify path. Getting that order wrong either way is a bug: polling after the finish path would request a pause on a cycle already declared over, and finishing without polling first is what made the closing pause wait on the next mutator allocation. The commit's other half -- the `debug_is_requested` rationale on `GCTrigger::poll` -- was already restored when comment-only divergence was reverted, so src/util/heap/gc_trigger.rs now differs from master only by this branch's `on_lazy_reclaim_finished` hook.
Upstream now carries the object log bit as an opt-in `lxr-object-log` feature (mmtk#1580, "LXR: use object log bit for probable write"), including the `RearmLoggedObjects` packet and the `FIRST_STW_STAGE` hand-off. That supersedes this branch's always-on implementation, so the conflicts resolve towards upstream. Resolutions: * barrier.rs -- took upstream's feature gating and its `flush_logged_objects` naming, dropping this branch's ungated `rearm_logged_objects`. Kept the branch-only `dec_origin` and `mapped_chunk` fields, which upstream does not have and `slot_has_unlog_bit` still uses, and kept the fuller rationale for why the object bit must be re-armed at the end of the epoch. * rc.rs, global.rs -- took upstream's gated form, keeping this branch's explanation of why promotion has to arm the bit, and the note that `Block::DEFRAG_STATE_TABLE` needs no explicit registration. * immortalspace.rs -- took upstream's `to_object_start::<VM>()` over this branch's `to_raw_address()`. The two differ wherever the object reference trails the object start, which is exactly the Julia case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge auto-resolved `set_side_metadata_for_chunk` by keeping both sides' `needs_field_log_bit` arm, so the bootimage's field unlog bits were set twice. Only upstream's rename of `_raw_start`/`_raw_size` to `raw_start`/`raw_size` turned that into a compile error; without it the duplicate would have merged silently and just done the work twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upstream absorbed the four LXR tuning knobs this branch carried as env vars (mmtk#1584) and renamed the object-log feature (mmtk#1583), so the branch's versions are dropped in favour of upstream's: - `inc_buffer_limit()`, `active_packet_split()`, `max_stw_sweep_nursery_blocks()` and `max_survival_mb()` are removed; their call sites now read `options.lxr_inc_buffer_limit`, `lxr_min_packet_split_size`/`lxr_min_packet_split_depth`, `lxr_max_stw_sweep_nursery_blocks` and `lxr_max_survival_mb`. - `SurvivalRatioPredictor::promotion_ratio()` is upstream's `promote_ratio()`, `ratio()` is `copy_promote_ratio()`, and `update_ratio()` is `update_ratios()`. The local predictor's single-argument `record_promotion` is superseded by upstream's `record_promotion(size, copied)`. - Feature `lxr-object-log` is now `lxr_object_log`. Note the defaults change with the knobs: upstream ships packet splitting off (`lxr_min_packet_split_size` = usize::MAX) and full in-pause nursery sweeping (`lxr_max_stw_sweep_nursery_blocks` = usize::MAX), where this branch defaulted to splitting at 64 slots and deferring all nursery sweeps. Both remain reachable via MMTK_* env vars. Kept branch-side: `dec_origin` and `mapped_chunk` on `LXRFieldBarrierSemantics`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`gc_start_time` had a single consumer: the elapsed time in the end-of-GC `info!` line. Master already records `pause_start_time` for exactly that, so the field was a leftover -- this restores master's version of the log and its comment, leaving global_state.rs diverging only by `set_concurrent_gc_finished`. The two are not quite the same interval: `gc_start_time` was stamped when the last parked worker picked up the GC goal, before `StopMutators`, so it counted the time to bring mutators to a halt; `pause_start_time` is stamped once they have all stopped. The line now reports pause time proper, which is what "End of Pause" means and what `record_pause_time` samples. `take_pause_time` runs well after this point (scheduler.rs), so the borrow here still finds the value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Master upstreamed this branch's Wake-N scheduler work (mmtk#1589), so the conflicting hunks in scheduler.rs and work_bucket.rs resolve to master's copy -- the code is the same, and master's doc comments are the accurate ones (`update_buckets` still said "Return true" here while returning `usize`). worker_monitor.rs keeps this branch's side, because mmtk#1589 took Wake-N without the machinery around it that only exists here: - `wake_n_while_locked` needs `&WorkerMonitorSync` so it can end with `notify_inactive_while_locked`. Master has no inactive workers to notify; this branch does (`set_active_workers`), and a worker parked on `active_worker_number_changed` is woken from nowhere else, so dropping that call would strand it for the rest of the pause. - `inactive_waiters`, the `sleeping` fast path and the `MMTK_WAKE_ALL` override are all branch-only and unchanged. `increase_inc_buffer_size` takes master's `fetch_update` + `saturating_add` (mmtk#1588), which fixes the lost-update bug this branch had fixed with `fetch_add` and additionally cannot wrap. The branch comment recording that measurement is reverted to master's, keeping the diff code-only; the finding moves to LXR_FINDINGS.md (untracked) as entry 10. mmtk#1586 (harness_begin) merged without conflict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Master's mmtk#1591 replaces the branch's lock-free wake-up fast path with its own: `WorkerParker` moves out of `WorkerMonitorSync` and `parked_workers` becomes an `AtomicUsize`, so `notify_work_available` can test it without taking the monitor lock. That supersedes this branch's `sleeping` counter, which existed only to be that lock-free reading and was strictly narrower -- `sleeping <= parked_workers` always, since a worker increments it only after `inc_parked_workers`. Master's counter is the more conservative test (it also counts the last parked worker while it runs `on_last_parked`), so it can cost an occasional redundant lock acquisition but cannot miss a wake-up. Dropped with it, both now judged not to earn their divergence: - `inactive_waiters`, and the `notify_inactive_while_locked` gate it fed. It only suppressed a `notify_all` on `active_worker_number_changed` when no worker was waiting there -- one no-op FUTEX_WAKE per bucket boundary, which is noise against a pause. The extra call it added to `wake_n_while_locked` was not load-bearing either: `set_active_workers` documents that its caller must follow with `notify_work_available(true)`, so a reactivation always arrives as `WakeAll`, never as `Wake(n)`. - `wake_all_override` (`MMTK_WAKE_ALL`), which existed to measure what the targeted wake-ups were worth. mmtk#1589 settled that. worker_monitor.rs is now byte-identical to master. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/global_state.rs # src/scheduler/scheduler.rs # src/vm/collection.rs
# Conflicts: # src/plan/lxr/gc_work/rc.rs
# Conflicts: # src/plan/lxr/gc_work/rc.rs # src/plan/lxr/global.rs
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
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.
A collection of LXR fixes for optimizing with Julia