Skip to content

chore(benchmarks): add benchmarks for niop opcode in all operation modes #3050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

rymnc
Copy link
Member

@rymnc rymnc commented Jul 2, 2025

This pull request introduces a new benchmarking group for niop operations in the alu.rs file. The changes add comprehensive benchmarks for various operations (ADD, SUB, MUL, EXP, SLL, XNOR) across different operand widths (U8, U16, U32)

Additions to benchmarking:

  • A new niop benchmarking group was added to evaluate the performance of narrow integer operations (niop) with different operand widths (U8, U16, U32).
  • Introduced the niop_bench macro to simplify the creation of benchmarks for operations like ADD, SUB, MUL, EXP, SLL, and XNOR.
  • Benchmarks iterate over generated operand pairs for each width, ensuring a variety of test cases for each operation.

@rymnc rymnc added the no changelog Skip the CI check of the changelog modification label Jul 2, 2025
@rymnc rymnc self-assigned this Jul 2, 2025
@rymnc rymnc marked this pull request as ready for review July 2, 2025 16:32
Copy link
Member

@MitchTurner MitchTurner left a comment

Choose a reason for hiding this comment

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

s there somewhere I can read up on what niop is? I've been out of the loop on it.

edit: The description helps haha:

A new niop benchmarking group was added to evaluate the performance of narrow integer operations (niop) with different operand widths (U8, U16, U32).

Still unfamiliar with what it's for, but this helps me approximate :)

@@ -201,6 +201,77 @@ pub fn run(c: &mut Criterion) {
.with_prepare_script(vec![op::movi(0x11, 100000)]),
);

{
let mut niop = c.benchmark_group("niop");
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think this would be more readable as niop_benchmark_group. It's confusing passing niop to something named niop_bench

Copy link
Member Author

Choose a reason for hiding this comment

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

addressed in ee93d38

Copy link
Member Author

@rymnc rymnc Jul 3, 2025

Choose a reason for hiding this comment

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

Copy link
Collaborator

@xgreenx xgreenx left a comment

Choose a reason for hiding this comment

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

Hmm, how we convert results of these benchmark into the actual cost? Because we have only one gas cost which we need to fill, but we have 18 benchmarks for it. I see how we can use it in sanity checks, but I don't understand how does it work for the collect binary=D

@rymnc
Copy link
Member Author

rymnc commented Jul 3, 2025

Hmm, how we convert results of these benchmark into the actual cost? Because we have only one gas cost which we need to fill, but we have 18 benchmarks for it. I see how we can use it in sanity checks, but I don't understand how does it work for the collect binary=D

checking

@rymnc
Copy link
Member Author

rymnc commented Jul 5, 2025

Hmm, how we convert results of these benchmark into the actual cost? Because we have only one gas cost which we need to fill, but we have 18 benchmarks for it. I see how we can use it in sanity checks, but I don't understand how does it work for the collect binary=D

the collect binary groups benchmark groups and takes the average from them i believe.

fn into_relative_costs(self, mut costs: Costs) -> Costs {
let baseline = self.get_baseline();
let State {
mut ids, groups, ..
} = self;
let relative = groups.into_iter().filter_map(|(name, samples)| {
let relative = samples
.into_iter()
.find(|sample| sample.starts_with(&name) && ids.contains_key(sample))
.and_then(|sample| ids.remove(&sample))
.map(|mean| Cost::Relative(map_to_ratio(baseline, mean)))?;
Some((name, relative))
});
costs.0.extend(relative);
costs
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Skip the CI check of the changelog modification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants