diff --git a/changelog.d/7972-gc-nursery-cap-budgeted-deferral.md b/changelog.d/7972-gc-nursery-cap-budgeted-deferral.md new file mode 100644 index 0000000000..57cfbc9a79 --- /dev/null +++ b/changelog.d/7972-gc-nursery-cap-budgeted-deferral.md @@ -0,0 +1,37 @@ +**fix(gc): a host safepoint no longer starts a budgeted cycle for a nursery-cap trigger it cannot discharge (#7909).** + +`gc_budgeted_due_trigger()` reported the young-generation scavenge cap as +`BudgetedGcTrigger::ArenaBytes`, so a host safepoint or mutator assist started a +**budgeted** cycle for it. A budgeted cycle is `low_pause_non_moving` by +construction — it sweeps in place and cannot lower +`copying_from_space_in_use_bytes()`, the exact quantity `young_scavenge_cap_due()` +tests — while `gc_safepoint_moving_minor` rejects every precise safepoint at its +`budgeted` entry guard for as long as the cycle is open. The two compose into a +self-sustaining stall (cap due → cycle started → moving minor locked out → +nothing reclaims → cap still due) that keeps the SATB mark barrier armed for the +rest of the process and reports **nothing**, because the `[gc]` trace is written +by the completion path. Measured on `gc-handoff/apps/asyncpipe.ts` when it was +filed: 1 cycle started, 15 steps, 0 completions, barrier armed 37 ms of a 127 ms +program, zero collections. + +The cap is now its own trigger variant, `YoungScavengeCap`. Every collection site +treats it identically to `ArenaBytes`; the split exists solely so the budgeted +stepper can tell them apart at the moment it decides whether to **start** a cycle. +When the cap is the only due trigger and the cycle would be budgeted, no cycle is +started and the pressure is deferred to the precise safepoint — arena baseline +included, so the `moving_defer_within_slack` valve is not left reading a stale, +already-exceeded baseline — exactly as `gc_check_trigger`'s alloc-point arm has +always done. That asymmetry between the two paths was the bug. No new knob; +`PERRY_GC_DIAG=1`'s `[gc-incremental]` line gains `nursery_cap_deferred=N` so the +refusal is distinguishable from "nothing was due". + +The regression test drives the real host-safepoint path with a per-thread cap +override and pairs the refusal with a control phase on the same fixture — same +thread, same heap, only the due trigger differs, and the control must still start +a cycle — so the pair discriminates "declines this trigger" from "declines +everything". It is a unit test rather than an end-to-end fixture because the +issue's named reproducer is not durable: `PERRY_GC_SCAVENGE_NURSERY_MB=4` on +`apps/asyncpipe.ts` reproduced the filed signature at 0.5.1490 and reproduces +nothing at 0.5.1495; swept across the whole dial the defect now presents one +notch lower, at cap 2, where a budgeted cycle is still started for the cap and +arms the mark barrier for 87.9 ms of a ~127 ms program. diff --git a/crates/perry-runtime/src/gc/instruments.rs b/crates/perry-runtime/src/gc/instruments.rs index 64bb2b31d0..25713e0ce5 100644 --- a/crates/perry-runtime/src/gc/instruments.rs +++ b/crates/perry-runtime/src/gc/instruments.rs @@ -216,12 +216,24 @@ pub(crate) enum BudgetedStepSkip { NoTrigger, StartBlocked, ResumeBlocked, + /// #7909: a cycle was NOT started because the only due trigger was the + /// young-generation scavenge cap, which a budgeted (low-pause non-moving) + /// cycle cannot discharge. The pressure is deferred to the precise + /// safepoint instead, where the copying minor can evacuate it. + /// + /// This is the one arm that is a *refusal*, not a stall: every other arm + /// leaves the trigger owned by a cycle that will eventually run, while this + /// one hands ownership to a different collector. It is counted because the + /// alternative — starting the cycle — is invisible in the `[gc]` trace, so + /// without a counter neither state can be told from "nothing was due". + NurseryCapUndischargeable, } static SKIP_REENTRANT: AtomicU64 = AtomicU64::new(0); static SKIP_NO_TRIGGER: AtomicU64 = AtomicU64::new(0); static SKIP_START_BLOCKED: AtomicU64 = AtomicU64::new(0); static SKIP_RESUME_BLOCKED: AtomicU64 = AtomicU64::new(0); +static SKIP_NURSERY_CAP_UNDISCHARGEABLE: AtomicU64 = AtomicU64::new(0); #[inline] pub(crate) fn note_budgeted_step_skip(reason: BudgetedStepSkip) { @@ -230,6 +242,7 @@ pub(crate) fn note_budgeted_step_skip(reason: BudgetedStepSkip) { BudgetedStepSkip::NoTrigger => &SKIP_NO_TRIGGER, BudgetedStepSkip::StartBlocked => &SKIP_START_BLOCKED, BudgetedStepSkip::ResumeBlocked => &SKIP_RESUME_BLOCKED, + BudgetedStepSkip::NurseryCapUndischargeable => &SKIP_NURSERY_CAP_UNDISCHARGEABLE, } .fetch_add(1, Ordering::Relaxed); } @@ -244,6 +257,12 @@ pub fn budgeted_step_skips() -> (u64, u64, u64, u64) { ) } +/// #7909: budgeted cycles declined because only the undischargeable young-gen +/// scavenge cap was due. Each one is a stalled cycle that did NOT happen. +pub fn budgeted_step_nursery_cap_deferrals() -> u64 { + SKIP_NURSERY_CAP_UNDISCHARGEABLE.load(Ordering::Relaxed) +} + // --------------------------------------------------------------------------- // #7903 — step-boundedness telemetry // diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index b1435bea50..028f25e5af 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -1164,7 +1164,8 @@ fn emit_incremental_liveness_diag() { "[gc-incremental] cycle_starts={} steps={} completions={} active_at_exit={} \ mark_barrier_arms={} mark_barrier_armed_us={} \ skips(reentrant={reentrant} no_trigger={no_trigger} start_blocked={start_blocked} \ - resume_blocked={resume_blocked}) safepoints_blocked_by_budgeted={} \ + resume_blocked={resume_blocked} nursery_cap_deferred={}) \ + safepoints_blocked_by_budgeted={} \ safepoints_blocked(in_alloc={blocked_alloc} unsafe_zone={blocked_unsafe_zone} \ root_lock={blocked_root_lock}) \ copying_minors={} loop_polls={} poll_arm_events={} \ @@ -1175,6 +1176,7 @@ fn emit_incremental_liveness_diag() { policy::gc_budgeted_cycle_active(), instruments::mark_barrier_arm_events(), instruments::mark_barrier_armed_us(), + instruments::budgeted_step_nursery_cap_deferrals(), instruments::moving_safepoints_blocked_by_budgeted(), instruments::copying_minor_cycles(), instruments::loop_polls_reached(), diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index fe2287d5c8..62a57d2ed3 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -108,8 +108,55 @@ pub(super) fn young_scavenge_cap_due() -> bool { if !nursery_cap_active() { return false; } - crate::arena::copying_from_space_in_use_bytes() - >= super::tenuring::scavenge_nursery_cap_effective_bytes() + crate::arena::copying_from_space_in_use_bytes() >= scavenge_nursery_cap_dueness_bytes() +} + +/// The cap value [`young_scavenge_cap_due`] compares against. +/// +/// Split out only so a test can make the cap due without allocating the real +/// 16 MB — a PER-THREAD override next to the reader, the shape support.rs +/// mandates for anything a test needs to move (never the process environment, +/// which is shared by every libtest thread; see #7946). It deliberately does +/// NOT feed `effective_next_arena_trigger`: this is about *dueness*, and a test +/// that also moved the trigger clamp would be changing two things at once. +fn scavenge_nursery_cap_dueness_bytes() -> usize { + #[cfg(test)] + if let Some(bytes) = GC_NURSERY_CAP_TEST_DUE_BYTES.with(Cell::get) { + return bytes; + } + super::tenuring::scavenge_nursery_cap_effective_bytes() +} + +#[cfg(test)] +thread_local! { + /// Test-only override for [`scavenge_nursery_cap_dueness_bytes`]. + static GC_NURSERY_CAP_TEST_DUE_BYTES: Cell> = const { Cell::new(None) }; +} + +/// RAII override making the young-gen scavenge cap due at `bytes` of from-space +/// occupancy on this thread (#7909). +#[cfg(test)] +pub(super) struct ScavengeNurseryCapTestGuard { + previous: Option, +} + +#[cfg(test)] +impl ScavengeNurseryCapTestGuard { + pub(super) fn due_at_bytes(bytes: usize) -> Self { + let previous = GC_NURSERY_CAP_TEST_DUE_BYTES.with(|cell| { + let previous = cell.get(); + cell.set(Some(bytes)); + previous + }); + Self { previous } + } +} + +#[cfg(test)] +impl Drop for ScavengeNurseryCapTestGuard { + fn drop(&mut self) { + GC_NURSERY_CAP_TEST_DUE_BYTES.with(|cell| cell.set(self.previous)); + } } /// Is the scavenge nursery cap in force? @@ -2344,7 +2391,14 @@ pub fn gc_check_trigger() { || super::roots::registered_root_scanners_block_budgeted_gc()) { let direct_kind = match gc_budgeted_due_trigger() { - Some(BudgetedGcTrigger::ArenaBytes) => Some(GcTriggerKind::ArenaBytes), + // #7909: `YoungScavengeCap` is a nursery-churn trigger exactly like + // `ArenaBytes` here — this arm's whole job is to route nursery + // pressure to a collection that can actually reclaim it, so the two + // must not diverge at THIS site. They diverge only at the budgeted + // stepper's start decision. + Some(BudgetedGcTrigger::ArenaBytes | BudgetedGcTrigger::YoungScavengeCap) => { + Some(GcTriggerKind::ArenaBytes) + } Some(BudgetedGcTrigger::MallocCount) => Some(GcTriggerKind::MallocCount), _ => None, }; @@ -2551,10 +2605,20 @@ struct BudgetedGcCycle { rebaseline: BudgetedGcRebaseline, } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] enum BudgetedGcTrigger { OldReclaim, ArenaBytes, + /// The young-generation scavenge cap ([`young_scavenge_cap_due`]). + /// + /// Split out of `ArenaBytes` by #7909. Every *collection* treats the two + /// identically — the split exists solely so the budgeted stepper can tell + /// them apart at the moment it decides whether to START a cycle, because + /// the quantity this one tests (`copying_from_space_in_use_bytes`) is one + /// a budgeted low-pause NON-MOVING cycle cannot lower. See + /// [`nursery_cap_active`] for why: a non-moving minor sweeps in place and + /// leaves from-space occupied. + YoungScavengeCap, MallocCount, } @@ -2612,7 +2676,7 @@ fn gc_budgeted_due_trigger() -> Option { return Some(BudgetedGcTrigger::ArenaBytes); } if young_scavenge_cap_due() { - return Some(BudgetedGcTrigger::ArenaBytes); + return Some(BudgetedGcTrigger::YoungScavengeCap); } let malloc_count = malloc_object_count(); @@ -2679,7 +2743,11 @@ pub(crate) fn gc_safepoint_moving_minor() -> bool { set_safepoint_pending(false); let _declared = DeclaredSafepointGuard::enter(); let kind = match gc_budgeted_due_trigger() { - Some(BudgetedGcTrigger::ArenaBytes) => GcTriggerKind::ArenaBytes, + // #7909: the nursery cap and the whole-arena trigger are the same + // collection here — this IS the evacuating collector the cap is for. + Some(BudgetedGcTrigger::ArenaBytes | BudgetedGcTrigger::YoungScavengeCap) => { + GcTriggerKind::ArenaBytes + } Some(BudgetedGcTrigger::MallocCount) => GcTriggerKind::MallocCount, // ★ #7148: old-gen reclaim used to be the alloc-point arm's business // exclusively — it ran a direct full mark-sweep behind a forced @@ -3127,7 +3195,10 @@ fn gc_start_budgeted_cycle_for_pressure(progress_kind: GcProgressKind) -> Option progress_kind, ) } - BudgetedGcTrigger::ArenaBytes => { + // #7909: identical treatment — a cycle that DOES start for nursery + // pressure (a non-budgeted one, which can evacuate) is the same + // arena-bytes collection it always was. + BudgetedGcTrigger::ArenaBytes | BudgetedGcTrigger::YoungScavengeCap => { let rebaseline = BudgetedGcRebaseline::ArenaBytes { pre_in_use: crate::arena::arena_in_use_bytes(), }; @@ -3300,6 +3371,19 @@ fn gc_budgeted_step_work_units_inner(work_units: usize) -> JsGcStepResult { gc_budgeted_step_work_units_inner_with_progress(work_units, GcProgressKind::NormalIncremental) } +/// #7909: arm the precise-root safepoint for nursery pressure the budgeted +/// stepper just declined, mirroring `gc_check_trigger`'s deferral arm exactly +/// (including the arena baseline the slack valve measures from — leaving that +/// stale would make `moving_defer_within_slack` read an already-exceeded +/// baseline and disable deferral for the rest of the process, the #7024 shape). +fn defer_nursery_cap_to_precise_safepoint() { + if GC_SAFEPOINT_PENDING.with(Cell::get) { + return; + } + GC_SAFEPOINT_DEFER_ARENA_BASE.with(|base| base.set(crate::arena::arena_total_bytes())); + set_safepoint_pending(true); +} + fn gc_budgeted_step_work_units_inner_with_progress( work_units: usize, start_progress_kind: GcProgressKind, @@ -3316,11 +3400,53 @@ fn gc_budgeted_step_work_units_inner_with_progress( }; if !gc_budgeted_cycle_active() { - if gc_budgeted_due_trigger().is_none() { + let Some(due) = gc_budgeted_due_trigger() else { super::instruments::note_budgeted_step_skip( super::instruments::BudgetedStepSkip::NoTrigger, ); return gc_idle_step_result(); + }; + if due == BudgetedGcTrigger::YoungScavengeCap && start_progress_kind.is_budgeted() { + // ★ #7909. Starting a budgeted cycle here is strictly worse than + // starting nothing, and it is self-sustaining. + // + // A budgeted cycle is `low_pause_non_moving` by construction + // (`progress_kind.is_budgeted()` at the collection site), so it + // sweeps in place and CANNOT lower + // `copying_from_space_in_use_bytes()` — the exact quantity + // `young_scavenge_cap_due()` tests. So the trigger it was started + // for survives the cycle. Worse, while the cycle is open + // `gc_safepoint_moving_minor` rejects every precise safepoint at + // its `budgeted` entry guard, so the ONE collector that can lower + // that quantity is locked out for the cycle's whole life. If the + // host's step cadence cannot finish the cycle — 2048 work units + // per microtask drain, and `asyncpipe` reaches ~15 drains after the + // cap goes due — the cycle never completes, is never cancelled, and + // the composition is permanent: cap due -> cycle started -> moving + // minor blocked -> nothing reclaims -> cap still due. The mutator + // then pays the SATB mark barrier for the rest of the process + // (measured: 22.9-42.5 ms of a ~127 ms program) for a collection + // that reclaims nothing, and the `[gc]` trace stays EMPTY because + // it is written by the completion path. + // + // The alloc-point arm already routes nursery pressure away from + // this stepper for the same reason (`gc_check_trigger`'s direct / + // deferred arm, which runs before the mutator assist). This is that + // asymmetry closed: the host-safepoint path now defers nursery + // pressure to the precise safepoint too, where the copying minor + // runs with rewritable roots and actually reclaims it. + // + // Note what is NOT skipped: `young_scavenge_cap_due()` is false + // unless `nursery_cap_active()`, which IS + // `gc_moving_loop_polls_enabled()`. So the cap can only be the due + // trigger in exactly the configuration where the precise route + // exists. When it does not, this branch is unreachable and the cap + // never fires at all. + super::instruments::note_budgeted_step_skip( + super::instruments::BudgetedStepSkip::NurseryCapUndischargeable, + ); + defer_nursery_cap_to_precise_safepoint(); + return gc_idle_step_result(); } if gc_budgeted_start_blocked() { super::instruments::note_budgeted_step_skip( diff --git a/crates/perry-runtime/src/gc/tests/host_safepoints.rs b/crates/perry-runtime/src/gc/tests/host_safepoints.rs index 66e07f8b91..743c49c015 100644 --- a/crates/perry-runtime/src/gc/tests/host_safepoints.rs +++ b/crates/perry-runtime/src/gc/tests/host_safepoints.rs @@ -314,12 +314,14 @@ fn host_safepoint_trace_reports_normal_incremental_budgeted_steps() { /// completions, still active at exit, mark barrier armed 37 ms of a 127 ms /// program, zero collections** — 11.8 % of the program's instructions. /// -/// This test does not assert the stall is *fixed* — it is not; the fix -/// (unblocking the moving minor) costs +51 % on that program because of the -/// per-cycle root-scanning price tracked in #7915. It pins the composition, so -/// that a change to either half is a deliberate change to a documented -/// interaction rather than a silent one, and it pins the instrument that makes -/// the state observable. +/// This test pins the **lockout half only**, on a trigger the budgeted cycle +/// CAN discharge (whole-arena bytes). That composition is intended: a cycle +/// that will finish owns the collector while it runs. What was the defect is +/// the *other* trigger — the young-gen scavenge cap, which no budgeted cycle +/// can lower — and that arm is closed by +/// `a_nursery_cap_only_trigger_is_deferred_to_the_collector_that_can_discharge_it` +/// below. Keep both: this one states what the lockout costs, that one states +/// when it is allowed to be paid. #[test] fn an_active_budgeted_cycle_locks_out_the_moving_minor_and_keeps_the_barrier_armed() { let _guard = CopyingNurseryTestGuard::new(1); @@ -385,3 +387,150 @@ fn an_active_budgeted_cycle_locks_out_the_moving_minor_and_keeps_the_barrier_arm ); assert!(!gc_budgeted_cycle_active()); } + +/// ★ #7909: a host safepoint must NOT start a budgeted cycle whose only due +/// trigger is the young-generation scavenge cap. +/// +/// # The defect this closes +/// +/// `young_scavenge_cap_due()` tests `copying_from_space_in_use_bytes()`. A +/// budgeted cycle is `low_pause_non_moving` by construction, sweeps in place, +/// and therefore **cannot lower that quantity** — so the trigger survives the +/// cycle it started. Meanwhile the cycle blocks `gc_safepoint_moving_minor` at +/// its `budgeted` entry guard, which is the one collector that *can* lower it. +/// If the host cadence cannot finish the cycle (2048 work units per microtask +/// drain; `gc-handoff/apps/asyncpipe.ts` reaches ~15 drains after the cap goes +/// due) the composition is permanent and completely silent: `cycle_starts=1 +/// steps=14 completions=0 active_at_exit=true`, the SATB mark barrier armed for +/// 22.9 ms of a 127 ms program, and **zero** `[gc]` lines because the trace is +/// written by the completion path. +/// +/// # Why this is a test and not a knob +/// +/// The defect is currently *unreached* on the shipped corpus — #7933/#7939 took +/// `asyncpipe`'s young survival to ~25‰, so the 16 MB cap never goes due there. +/// That is a property of one week's allocation profile, not of the collector: +/// `PERRY_GC_SCAVENGE_NURSERY_MB=4` reproduces the original signature exactly on +/// today's `main`. Closing the issue on those numbers would delete the knowledge +/// and let the next allocation-rate change silently re-expose it. +/// +/// # Why the control phase is not optional +/// +/// "No cycle was started" is satisfied by a fixture where nothing was due at +/// all, which is the failure mode this repo keeps paying for. So phase 1 asserts +/// the cap is due *and* that neither other trigger is, and phase 2 then makes +/// the whole-arena trigger due **on the same thread, with the same heap** and +/// asserts a cycle DOES start. Only the trigger differs between the two phases, +/// so the pair discriminates "declines this trigger" from "declines everything". +#[test] +fn a_nursery_cap_only_trigger_is_deferred_to_the_collector_that_can_discharge_it() { + let _guard = CopyingNurseryTestGuard::new(1); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_old_reclaim_pressure(); + + // Real young-gen occupancy, then a cap that the occupancy clears. The + // override is per-thread and next to its reader (support.rs's rule); moving + // `PERRY_GC_SCAVENGE_NURSERY_MB` would be a process-wide environment write. + let live = live_test_string(b"nursery_cap_7909_live"); + js_shadow_slot_set(0, string_bits(live)); + for _ in 0..6_000 { + let _ = young_leaf(); + } + let _cap = super::super::policy::ScavengeNurseryCapTestGuard::due_at_bytes(1); + + // ── phase 1: cap-only pressure ─────────────────────────────────────────── + // ★ Live subject: the cap is genuinely due... + assert!( + crate::arena::copying_from_space_in_use_bytes() > 0, + "fixture must have young-gen occupancy for the cap to test" + ); + assert!( + super::super::policy::young_scavenge_cap_due(), + "fixture must present a due young-gen scavenge cap" + ); + // ...and it is the ONLY thing due, so the refusal below can only be about it. + assert!( + crate::arena::arena_total_bytes() < super::super::policy::next_arena_trigger_base(), + "the whole-arena trigger must NOT be due in phase 1" + ); + assert!( + malloc_object_count() < GC_NEXT_MALLOC_TRIGGER.with(|trigger| trigger.get()), + "the malloc trigger must NOT be due in phase 1" + ); + + let starts_before = super::super::instruments::incremental_cycle_starts(); + let deferrals_before = super::super::instruments::budgeted_step_nursery_cap_deferrals(); + + let declined = gc_runtime_safepoint(); + + assert_eq!( + declined.status, JS_GC_STEP_STATUS_IDLE, + "a cap-only host safepoint must report idle, not an active cycle" + ); + assert_eq!( + super::super::instruments::incremental_cycle_starts(), + starts_before, + "no budgeted cycle may be started for a trigger it cannot discharge" + ); + assert!( + !gc_budgeted_cycle_active(), + "no budgeted cycle may be left active by a cap-only safepoint" + ); + // ★ The refusal is attributed, not merely absent: without this the assertion + // above is also satisfied by every other reason a start can be skipped + // (reentrant, start-blocked, nothing due). + assert_eq!( + super::super::instruments::budgeted_step_nursery_cap_deferrals(), + deferrals_before + 1, + "the refusal must be counted as the nursery-cap deferral specifically" + ); + // The deferral hands the pressure to the precise safepoint, exactly as the + // alloc-point arm does — otherwise a compute loop would never poll for it. + assert!( + GC_SAFEPOINT_PENDING.with(|pending| pending.get()), + "declining the cycle must arm the precise safepoint instead" + ); + + // ★ The point of the whole fix: the collector that CAN discharge the cap is + // reachable. Under the defect this returns false, for the `budgeted` reason. + let blocked_before = super::super::instruments::moving_safepoints_blocked_by_budgeted(); + assert!( + super::super::gc_safepoint_moving_minor(), + "the moving minor must not be locked out by a cycle that was never started" + ); + assert_eq!( + super::super::instruments::moving_safepoints_blocked_by_budgeted(), + blocked_before, + "no safepoint may be rejected for `budgeted` when no cycle is active" + ); + + // ── phase 2 (control): the SAME fixture, a discharge-able trigger ──────── + // Same thread, same heap, same guards — only the due trigger differs. A + // cycle must start here, or phase 1 proves nothing. + trigger_guard.make_arena_trigger_due(); + assert!( + crate::arena::arena_total_bytes() >= super::super::policy::next_arena_trigger_base(), + "control phase must present a due whole-arena trigger" + ); + + let started = gc_runtime_safepoint(); + assert_eq!( + started.status, JS_GC_STEP_STATUS_ACTIVE, + "a whole-arena trigger IS discharge-able by a budgeted cycle and must still start one" + ); + assert_eq!( + super::super::instruments::incremental_cycle_starts(), + starts_before + 1, + "the control must start exactly one cycle" + ); + assert_eq!( + super::super::instruments::budgeted_step_nursery_cap_deferrals(), + deferrals_before + 1, + "the control must NOT be counted as a nursery-cap deferral" + ); + + // Leave the shared thread state clean. + let completed = complete_host_safepoint_cycle(); + assert_eq!(completed.status, JS_GC_STEP_STATUS_COMPLETED); + assert!(!gc_budgeted_cycle_active()); +}