Skip to content

Conversation

@AkliluYirgalem
Copy link

groundwork for multi-program benchmarking: Added a file_name field to Mollusk Struct. This allows the bencher crate to track individual program filename, which i will use in the next PR to implement multi-program benchmarking and the filename will be used in reporting the Markdown file.

Copy link
Collaborator

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

This relationship is a little misleading if we put the file name on the Mollusk struct. A Mollusk instance houses a program cache, which can hold many programs and be reused to invoke multiple programs at the top-level, especially over transaction contexts and instruction chains.

Is there a way we can refactor the bencher to support file name distinguishing instead?

@AkliluYirgalem
Copy link
Author

Yes, you are right, that was an oversimplification of naming a program, i immediately thought of a list of file names but thats also an overcomplicating solution.

about refactoring the bencher, i can think of two solutions, one is accepting a generic param from the new function and converting their types using the From trait(by implementing it in Mollusk), into Vec of tuples, like (mollusk, file_name), that will preserve backward compatibility by giving default file_name for uninformed tests.
the other is by just accepting a vector of tuples, much like bench, this will break old tests.

a snapshot for the first one:

//for backward compatability
impl From<Mollusk> for Vec<(Mollusk, String)> {
    fn from(m: Mollusk) -> Self {
        vec![(m, String::new("default_file_name"))] 
    }
}

//for the newer inputs
impl From<(Mollusk, &str)> for Vec<(Mollusk, String)> {
    fn from((m, s): (Mollusk, &str)) -> Self {
        vec![(m, s.to_string())]
    }
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants