Skip to content

Commit 4d785bb

Browse files
authored
Remove quadratic behavior cloning rec groups (#1952)
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.
1 parent 00091b2 commit 4d785bb

File tree

1 file changed

+13
-25
lines changed

1 file changed

+13
-25
lines changed

crates/wasm-smith/src/core.rs

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ impl Module {
610610

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

@@ -640,38 +640,26 @@ impl Module {
640640
Ok(())
641641
}
642642

643-
/// Returns an iterator of rec groups that we could currently clone while
644-
/// still staying within the max types limit.
645-
fn clonable_rec_groups(
646-
&self,
647-
kind: AllowEmptyRecGroup,
648-
) -> impl Iterator<Item = Range<usize>> + '_ {
649-
self.rec_groups
650-
.iter()
651-
.filter(move |r| {
652-
match kind {
653-
AllowEmptyRecGroup::Yes => {}
654-
AllowEmptyRecGroup::No => {
655-
if r.is_empty() {
656-
return false;
657-
}
658-
}
659-
}
660-
r.end - r.start <= self.config.max_types.saturating_sub(self.types.len())
661-
})
662-
.cloned()
663-
}
664-
665643
fn clone_rec_group(&mut self, u: &mut Unstructured, kind: AllowEmptyRecGroup) -> Result<()> {
644+
// Choose an arbitrary rec group to clone, but bail out if the selected
645+
// rec group isn't valid to clone. For example if empty groups aren't
646+
// allowed and the selected group is empty, or if cloning the rec group
647+
// would cause the maximum number of types to be exceeded.
648+
let group = u.choose(&self.rec_groups)?.clone();
649+
if group.is_empty() && kind == AllowEmptyRecGroup::No {
650+
return Ok(());
651+
}
652+
if group.len() > self.config.max_types.saturating_sub(self.types.len()) {
653+
return Ok(());
654+
}
655+
666656
// NB: this does *not* guarantee that the cloned rec group will
667657
// canonicalize the same as the original rec group and be deduplicated.
668658
// That would require a second pass over the cloned types to rewrite
669659
// references within the original rec group to be references into the
670660
// new rec group. That might make sense to do one day, but for now we
671661
// don't do it. That also means that we can't mark the new types as
672662
// "subtypes" of the old types and vice versa.
673-
let candidates: Vec<_> = self.clonable_rec_groups(kind).collect();
674-
let group = u.choose(&candidates)?.clone();
675663
let new_rec_group_start = self.types.len();
676664
for index in group {
677665
let orig_ty_index = u32::try_from(index).unwrap();

0 commit comments

Comments
 (0)