Skip to content

Commit

Permalink
Remove quadratic behavior cloning rec groups (bytecodealliance#1952)
Browse files Browse the repository at this point in the history
The call to `clone_rec_group` would create a new vector each time it
was called of all clonable rec groups where if an empty rec group is
cloned this creates quadratic behavior where each time a vector is
created and one more item is appended.

The fix in this commit is to avoid creating a precise set of candidates
to clone and just throwing out candidates that aren't applicable.
  • Loading branch information
alexcrichton authored and mjoerussell committed Dec 17, 2024
1 parent 57f4e91 commit 9366531
Showing 1 changed file with 13 additions and 25 deletions.
38 changes: 13 additions & 25 deletions crates/wasm-smith/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ impl Module {

if self.config.gc_enabled {
// With small probability, clone an existing rec group.
if self.clonable_rec_groups(kind).next().is_some() && u.ratio(1, u8::MAX)? {
if self.rec_groups.len() > 0 && u.ratio(1, u8::MAX)? {
return self.clone_rec_group(u, kind);
}

Expand Down Expand Up @@ -640,38 +640,26 @@ impl Module {
Ok(())
}

/// Returns an iterator of rec groups that we could currently clone while
/// still staying within the max types limit.
fn clonable_rec_groups(
&self,
kind: AllowEmptyRecGroup,
) -> impl Iterator<Item = Range<usize>> + '_ {
self.rec_groups
.iter()
.filter(move |r| {
match kind {
AllowEmptyRecGroup::Yes => {}
AllowEmptyRecGroup::No => {
if r.is_empty() {
return false;
}
}
}
r.end - r.start <= self.config.max_types.saturating_sub(self.types.len())
})
.cloned()
}

fn clone_rec_group(&mut self, u: &mut Unstructured, kind: AllowEmptyRecGroup) -> Result<()> {
// Choose an arbitrary rec group to clone, but bail out if the selected
// rec group isn't valid to clone. For example if empty groups aren't
// allowed and the selected group is empty, or if cloning the rec group
// would cause the maximum number of types to be exceeded.
let group = u.choose(&self.rec_groups)?.clone();
if group.is_empty() && kind == AllowEmptyRecGroup::No {
return Ok(());
}
if group.len() > self.config.max_types.saturating_sub(self.types.len()) {
return Ok(());
}

// NB: this does *not* guarantee that the cloned rec group will
// canonicalize the same as the original rec group and be deduplicated.
// That would require a second pass over the cloned types to rewrite
// references within the original rec group to be references into the
// new rec group. That might make sense to do one day, but for now we
// don't do it. That also means that we can't mark the new types as
// "subtypes" of the old types and vice versa.
let candidates: Vec<_> = self.clonable_rec_groups(kind).collect();
let group = u.choose(&candidates)?.clone();
let new_rec_group_start = self.types.len();
for index in group {
let orig_ty_index = u32::try_from(index).unwrap();
Expand Down

0 comments on commit 9366531

Please sign in to comment.