From 427e041f1c189b114552d10cbc9aa11204b5db99 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 3 Jun 2022 16:01:10 -0700 Subject: [PATCH] Fix spillslot allocation to actually reuse spillslots. (#56) * Fix spillslot allocation to actually reuse spillslots. The old logic, which did some linked-list rearranging to try to probe more-likely-to-be-free slots first and which was inherited straight from the original IonMonkey allocator, was slightly broken (error in translation and not in IonMonkey, to be clear): it did not get the list-splicing right, so quite often dropped a slot on the floor and failed to consider it for further reuse. On some experimentation, it seems to work just as well to keep a SmallVec of spillslot indices per size class instead, and save the last probe-point in order to spread load throughout the allocated slots while limiting the number of probes (to bound quadratic behavior). This change moves the maximum slot count from 285 to 92 in `python.wasm` from bytecodealliance/wasmtime#4214, and the maximum frame size from 2384 bytes to 752 bytes. --- src/ion/data_structures.rs | 18 ++++++++-- src/ion/spill.rs | 67 +++++++++++++------------------------- 2 files changed, 38 insertions(+), 47 deletions(-) diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index dd1013a..5c986bb 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -439,13 +439,25 @@ pub struct SpillSlotData { pub ranges: LiveRangeSet, pub class: RegClass, pub alloc: Allocation, - pub next_spillslot: SpillSlotIndex, } #[derive(Clone, Debug)] pub struct SpillSlotList { - pub first_spillslot: SpillSlotIndex, - pub last_spillslot: SpillSlotIndex, + pub slots: SmallVec<[SpillSlotIndex; 32]>, + pub probe_start: usize, +} + +impl SpillSlotList { + /// Get the next spillslot index in probing order, wrapping around + /// at the end of the slots list. + pub(crate) fn next_index(&self, index: usize) -> usize { + debug_assert!(index < self.slots.len()); + if index == self.slots.len() - 1 { + 0 + } else { + index + 1 + } + } } #[derive(Clone, Debug)] diff --git a/src/ion/spill.rs b/src/ion/spill.rs index 2600cdb..45a2716 100644 --- a/src/ion/spill.rs +++ b/src/ion/spill.rs @@ -17,6 +17,7 @@ use super::{ SpillSetIndex, SpillSlotData, SpillSlotIndex, SpillSlotList, }; use crate::{Allocation, Function, SpillSlot}; +use smallvec::smallvec; impl<'a, F: Function> Env<'a, F> { pub fn try_allocating_regs_for_spilled_bundles(&mut self) { @@ -110,6 +111,8 @@ impl<'a, F: Function> Env<'a, F> { } pub fn allocate_spillslots(&mut self) { + const MAX_ATTEMPTS: usize = 10; + for spillset in 0..self.spillsets.len() { trace!("allocate spillslot: {}", spillset); let spillset = SpillSetIndex::new(spillset); @@ -122,70 +125,46 @@ impl<'a, F: Function> Env<'a, F> { self.slots_by_size.resize( size + 1, SpillSlotList { - first_spillslot: SpillSlotIndex::invalid(), - last_spillslot: SpillSlotIndex::invalid(), + slots: smallvec![], + probe_start: 0, }, ); } // Try a few existing spillslots. - let mut spillslot_iter = self.slots_by_size[size].first_spillslot; - let mut first_slot = SpillSlotIndex::invalid(); - let mut prev = SpillSlotIndex::invalid(); + let mut i = self.slots_by_size[size].probe_start; let mut success = false; - for _attempt in 0..10 { - if spillslot_iter.is_invalid() { - break; - } - if spillslot_iter == first_slot { - // We've started looking at slots we placed at the end; end search. - break; - } - if first_slot.is_invalid() { - first_slot = spillslot_iter; - } + // Never probe the same element more than once: limit the + // attempt count to the number of slots in existence. + for _attempt in 0..std::cmp::min(self.slots_by_size[size].slots.len(), MAX_ATTEMPTS) { + // Note: this indexing of `slots` is always valid + // because either the `slots` list is empty and the + // iteration limit above consequently means we don't + // run this loop at all, or else `probe_start` is + // in-bounds (because it is made so below when we add + // a slot, and it always takes on the last index `i` + // after this loop). + let spillslot = self.slots_by_size[size].slots[i]; - if self.spillslot_can_fit_spillset(spillslot_iter, spillset) { - self.allocate_spillset_to_spillslot(spillset, spillslot_iter); + if self.spillslot_can_fit_spillset(spillslot, spillset) { + self.allocate_spillset_to_spillslot(spillset, spillslot); success = true; + self.slots_by_size[size].probe_start = i; break; } - // Remove the slot and place it at the end of the respective list. - let next = self.spillslots[spillslot_iter.index()].next_spillslot; - if prev.is_valid() { - self.spillslots[prev.index()].next_spillslot = next; - } else { - self.slots_by_size[size].first_spillslot = next; - } - if !next.is_valid() { - self.slots_by_size[size].last_spillslot = prev; - } - let last = self.slots_by_size[size].last_spillslot; - if last.is_valid() { - self.spillslots[last.index()].next_spillslot = spillslot_iter; - } else { - self.slots_by_size[size].first_spillslot = spillslot_iter; - } - self.slots_by_size[size].last_spillslot = spillslot_iter; - - prev = spillslot_iter; - spillslot_iter = next; + i = self.slots_by_size[size].next_index(i); } if !success { // Allocate a new spillslot. let spillslot = SpillSlotIndex::new(self.spillslots.len()); - let next = self.slots_by_size[size].first_spillslot; self.spillslots.push(SpillSlotData { ranges: LiveRangeSet::new(), - next_spillslot: next, alloc: Allocation::none(), class: self.spillsets[spillset.index()].class, }); - self.slots_by_size[size].first_spillslot = spillslot; - if !next.is_valid() { - self.slots_by_size[size].last_spillslot = spillslot; - } + self.slots_by_size[size].slots.push(spillslot); + self.slots_by_size[size].probe_start = self.slots_by_size[size].slots.len() - 1; self.allocate_spillset_to_spillslot(spillset, spillslot); }