From f0f855e99885a28c77bd4a5b82391a5c717aca04 Mon Sep 17 00:00:00 2001 From: Zac Miller Date: Mon, 11 May 2026 15:33:54 -0400 Subject: [PATCH 1/2] perf(system): eliminate two Planetesimal clones in planetesimals_intersect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Roche-limit computation in planetesimals_intersect cloned both `p` and `prev_p` purely to satisfy the borrow checker — only three scalar fields (mass, mass, radius) are read from the clones. Planetesimal carries Vec moons + Vec rings, so each clone is a deep tree copy. In hot post-accretion loops this is per-iteration waste. Replace the two clones with direct scalar extraction: let (larger_mass, smaller_mass, smaller_radius) = if p.mass >= prev_p.mass { (p.mass, prev_p.mass, prev_p.radius) } else { (prev_p.mass, p.mass, p.radius) }; Note the third argument: the original code passed `&smaller.radius` (the smaller body's radius, matching roche_limit_au's documented contract that the third arg is the moon/smaller body's radius). The refactor preserves this — using smaller_radius rather than the larger body's radius keeps the physics identical. For the capture_moon call site, use std::mem::swap with a `>=` tiebreak to orient the bodies in place: after the swap, prev_p holds the larger body. This matches the original `match p.mass >= prev_p.mass { ... }` ordering exactly (p wins on equal mass). All 16 fixture tests remain bit-identical after the refactor — zero fixture drift under cargo test, as the clone-elimination itself does not change the arithmetic order. --- src/lib.rs | 9 +++++++++ src/structs/system.rs | 22 ++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ed215e1..741754b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -231,4 +231,13 @@ mod tests { let system = format!("{:?}", accrete.planet()); assert_eq!(system, fixture); } + + #[test] + fn same_seed_same_system_after_fix_3a() { + let mut a1 = Accrete::new(1); + let s1 = a1.planetary_system(); + let mut a2 = Accrete::new(1); + let s2 = a2.planetary_system(); + assert_eq!(format!("{:?}", s1), format!("{:?}", s2)); + } } diff --git a/src/structs/system.rs b/src/structs/system.rs index 68757b9..e6b2d74 100644 --- a/src/structs/system.rs +++ b/src/structs/system.rs @@ -254,17 +254,27 @@ pub fn planetesimals_intersect( if p.is_moon { *prev_p = coalesce_two_planets(prev_p, p, events_log); } else { - // Check for larger/smaller planetesimal - let (larger, smaller) = match p.mass >= prev_p.mass { - true => (p.clone(), prev_p.clone()), - false => (prev_p.clone(), p.clone()), + // Compute roche_limit from scalars — avoids deep-cloning both bodies + // (each carries Vec moons + Vec rings) just to read + // three f64s. The original code cloned `larger` and `smaller` purely to + // satisfy the borrow checker. Third arg to roche_limit_au is the + // smaller/moon body's radius (see roche_limit_au doc in utils.rs). + let (larger_mass, smaller_mass, smaller_radius) = if p.mass >= prev_p.mass { + (p.mass, prev_p.mass, prev_p.radius) + } else { + (prev_p.mass, p.mass, p.radius) }; - let roche_limit = roche_limit_au(&larger.mass, &smaller.mass, &smaller.radius); + let roche_limit = roche_limit_au(&larger_mass, &smaller_mass, &smaller_radius); // Planetesimals collide or one capture another as moon if (prev_p.a - p.a).abs() <= roche_limit * 2.0 { *prev_p = coalesce_two_planets(prev_p, p, events_log); } else { - *prev_p = capture_moon(&larger, &smaller, primary_star_mass, rng, events_log); + // Ensure prev_p holds the larger body before capture_moon. Use + // `>=` to preserve the original tiebreak (p wins on equal mass). + if p.mass >= prev_p.mass { + std::mem::swap(p, prev_p); + } + *prev_p = capture_moon(prev_p, p, primary_star_mass, rng, events_log); prev_p .moons .sort_by(|p1, p2| p1.a.partial_cmp(&p2.a).unwrap()); From 93c484bf296b51b1839bc09f5abeca6f7b989c60 Mon Sep 17 00:00:00 2001 From: Zac Miller Date: Tue, 12 May 2026 15:09:02 -0400 Subject: [PATCH 2/2] fix(system): reborrow events_log on capture_moon call after master merge The merge of master (post PR 1 + PR 2) changed events_log from &mut AccreteEvents to Option<&mut AccreteEvents>. The capture_moon call site at planetesimals_intersect was missed during conflict resolution and moved the Option instead of reborrowing it, leaving the subsequent coalesce_planetesimals and moons_to_rings calls trying to use a moved value. Add .as_deref_mut() to match the surrounding calls. --- src/structs/system.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/structs/system.rs b/src/structs/system.rs index cdb6db1..0234712 100644 --- a/src/structs/system.rs +++ b/src/structs/system.rs @@ -274,7 +274,7 @@ pub fn planetesimals_intersect( if p.mass >= prev_p.mass { std::mem::swap(p, prev_p); } - *prev_p = capture_moon(prev_p, p, primary_star_mass, rng, events_log); + *prev_p = capture_moon(prev_p, p, primary_star_mass, rng, events_log.as_deref_mut()); prev_p .moons .sort_by(|p1, p2| p1.a.partial_cmp(&p2.a).unwrap());