Skip to content

Conversation

@DaniPopes
Copy link
Member

Closes #8898.

Continuation of #11769 + #11842. See these PRs for more details.

@DaniPopes DaniPopes added Cmd-forge-test Command: forge test T-perf Type: performance labels Dec 10, 2025

let corpus_entry = CorpusEntry::new_existing(tx_seq.to_vec(), entry.path.clone())?;
self.in_memory_corpus.push(corpus_entry);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this rm corpus files that didn't bring new coverage?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

/// Syncs and calibrates the in memory corpus and updates the history_map if new coverage is
/// found from the corpus findings of other workers.
#[instrument(skip_all)]
fn calibrate(
Copy link
Contributor

@0xalpharush 0xalpharush Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From #11769 (comment)

AFL ++ stores a bit map of the history_map (1-byte hitcount -> bool indicating hit) for each input and then uses this bit map to make sure it doesn't eject an entry that is unique (this would mean updating is_favored to mean it contributes a unique bit to the bitmap).

I think here we need to add a minimize step that finds the smallest set of inputs that cover the intersection of all the edges. Really, a worker should replace the in-memory corpus element if we find a smaller sequence or it finds one sequence that is the super set of N sequences. This would apply to both while fuzzing (prior to exporting to master) and also while syncing (master has a smaller seq.)

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, left some initial thoughts, still going through / test

};
/// Determines the number of workers to run.
fn num_workers(&self) -> usize {
rayon::current_num_threads()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make this a config so we could set number of workers per test? and maybe default to 1 to preserve current behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like threads to be a configurable value other than CLI -jX. this is an implementation detail and any weird behavior should be tackled in the implementation not with a config that disables it

Copy link
Member Author

@DaniPopes DaniPopes Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example adding a minimum runs per worker f90ee86

//!
//! ## Workflow
//!
//! - Each worker maintains its own local corpus with entries stored as JSON files
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you see any issue if we implement the workers sync mechanism with an in-memory database / notify channel instead files and dump on disk only the interesting corpus like we do now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cmd-forge-test Command: forge test T-perf Type: performance

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

forge test doesn't utilize all available threads for fuzzing/invariants

5 participants