Add benchmark for simple transaction

[Fumuran]

This PR implements the benchmark of the default transaction with default (mock) notes and with P2ID note.
This benchmark prints the number of cycles required to execute each stage of the transaction.

[Fumuran]

Although this solution works, there are some things which I don't like. Namely:

1. I don't like the fact that I need to modify the "core" main.masm just for the benchmark. And not only just with harmless emit instructions: the problem with decorators being used in an empty span is appeared again, so I have to add some kind of push.0 drop instructions just to put something to the span block. I thought that I can use the modified copy of the main.masm file in the test, but it seems that we are using this file too early in the pipeline to be able to change it during the test.
2. It is a little bit strange for me to implement benchmark as a test. It doesn't really test anything and it requires additional time and resources while testing the whole project. Looks like it is better to make it as an "example", although I'm not sure how can we do that in this repo.
3. For now I just print the current cycle to the stdout using println!, which I have to import from the standard library. I tried to avoid using std, but didn't find a way to output some data without it. Probably we can use some logger? But it looks like an overkill for the miden-base to have it. But maybe I'm wrong.
4. Probably I should make the output message more useful: now I am printing only the cycle count and a context, but since we have access to the Host, we could print something more meaningful, I just not sure what information could be useful.

[bobbinth]

Without having looked at the code too much, a few thoughts:

1. I don't like the fact that I need to modify the "core" main.masm just for the benchmark. And not only just with harmless emit instructions: the problem with decorators being used in an empty span is appeared again, so I have to add some kind of push.0 drop instructions just to put something to the span block. I thought that I can use the modified copy of the main.masm file in the test, but it seems that we are using this file too early in the pipeline to be able to change it during the test.

I don't think that's an issue. The need for push.0 drop is indeed annoying but that's a separate problem, and hopefully, we'll find a solution soon.

2. It is a little bit strange for me to implement benchmark as a test. It doesn't really test anything and it requires additional time and resources while testing the whole project. Looks like it is better to make it as an "example", although I'm not sure how can we do that in this repo.

I probably wouldn't implement them as a test (especially since tests are usually not run in release mode). One other option is to write this as a small binary program which one can run via one of the cargo make commands. But maybe others have other suggestions too.

3. For now I just print the current cycle to the stdout using println!, which I have to import from the standard library. I tried to avoid using std, but didn't find a way to output some data without it. Probably we can use some logger? But it looks like an overkill for the miden-base to have it. But maybe I'm wrong.

I don't think usage of std for benchmark purposes is an issue - but I'd tried to avoid using it inside TransactionHost. I'm not sure what's the best solution here, but you can try to wrap TransactionHost in a new host which overrides the relevant functions and prints to stdout. Another option is to collect benchmarking info in some struct of transaction host, and fill this struct only if some flag in the host is set. But the wrapper approach would be preferred if it works.

4. Probably I should make the output message more useful: now I am printing only the cycle count and a context, but since we have access to the Host, we could print something more meaningful, I just not sure what information could be useful.

Why aren't we using trace instruction for this (instead of emit)? I would use it and instead of having a single new event would have trace events for every stage we are trying to benchmark.

[bobbinth]

I would use the same scheme here as we do for events and error codes - i.e., the first 16 bits are set to 2 and the remaining 16 bits is a counter that increments (0, 1, 2 etc.).

[Fumuran]

I didn't manage to create a binary program from this test. Is there a way to do that without additional libraries?

[bobbinth]

Looks good! Thank you! I added some comments inline. One thing I'm starting to think is that maybe we should move capturing of the tracing info into the main TransactionHost and bench host would be responsible only for printing it out.

[bobbinth]

I didn't manage to create a binary program from this test. Is there a way to do that without additional libraries?

We don't need a new library, but we would need to create a new binary (a Rust project can have many binaries).

Btw, what are the cycle counts look like for some simple transaction?

[Fumuran]

Currently for the default empty transaction we have this result:

• Number of cycles it takes to execute the prologue: 3642
• Number of cycles it takes to execute the note processing: 1141
• Number of cycles it takes to execute the transaction script processing: 30
• Number of cycles it takes to execute the epilogue: 2230

[Fumuran]

The benchmark could be ran with this command:

cargo make bench-simple

Currently the output looks like so:

Number of cycles it takes to execule:
- Prologue: 3642,
- Notes processing: 1149,
--- Note #0: 713
--- Note #1: 390
- Transaction script processing: 30,
- Epilogue: 2230

[bobbinth]

Currently the output looks like so:

Number of cycles it takes to execule:
- Prologue: 3642,
- Notes processing: 1149,
--- Note #0: 713
--- Note #1: 390
- Transaction script processing: 30,
- Epilogue: 2230

Could we do this as a transaction which consumes a P2ID note into a basic wallet? The main difference would be is that transaction script would need to include a signature and would probably take about 80K cycles.

[bobbinth]

Thank you! Looks good! Not a full review - but I left some more comments inline.

[Fumuran]

Benchmarks could be executed with these commands:

• Default notes:

  cargo make bench-tx
  

  Result of this benchmark:

  Number of cycles it takes to execule:
  - Prologue: 3642,
  - Notes processing: 1149,
  --- Note 0x8a55c3531cdd5725aa805475093ed3006c6773b71a008e8ca840da8364a67cd6: 713
  --- Note 0xae832786b774651bb0e5b71615fcf9f01427c054a8919cda6d2fd4a0c8948167: 390
  - Transaction script processing: 30,
  - Epilogue: 2230
  

• P2ID note:

  cargo make bench-p2id
  

  Result of this benchmark:

  Number of cycles it takes to execule:
  - Prologue: 2002,
  - Notes processing: 918,
  --- Note 0xb9fa30eb43d80d579be02dc004338e06b5ad565e81e0bac11a94ab01abfdd40a: 881
  - Transaction script processing: 88207,
  - Epilogue: 280
  

[bobbinth]

Thank you! Looks good! I left a few more comments inline.

[bobbinth]

In case we haven't start executing any note, expect will panic.

What about after note execution is done? In general, I still think it would be better to check the value of the current note pointer.

I wanted to return an ExecutionError, but looks like we don't have an error for the cases when the memory segment wasn't initialized. I can implement it, but I'm not sure that it is worth it since it is used only here.

I think returning Option (the way you have it now) is fine.

[bobbinth]

Hmmm - I would have thought that something like this should work:

[[bin]]
name = "bench-tx"
path = "src/main.rs"

[dependencies]
miden-lib = { package = "miden-lib", path = "../miden-lib", version = "0.3" }
miden-objects = { package = "miden-objects", path = "../objects", version = "0.3" }
miden-tx = { package = "miden-tx", path = "../miden-tx", version = "0.3" }
mock = { package = "miden-mock", path = "../mock"  }
serde_json = { package = "serde_json", version = "1.0" }
vm-processor = { workspace = true, features = ["std"] }

But I didn't test it.

Either way - I think we should try to figure this out as it may mean that there is something wrong somewhere.

[bobbinth]

I think the struct could look something like:

pub struct TransactionBenchmark {
    prologue: u32,
    notes_processing: u32,
    note_execution: Vec<(String, u32)>,
    tx_script_processing: u32,
    epilogue: u32,
}

And then we can implement TryFrom<TransactionProgress> for TransactionBenchmark. This conversion should be pretty straight-forward.

[Fumuran]

@bobbinth @hackaugusto Yesterday I spent the whole day trying to reconstruct the JSON file to be able to track the changes commit by commit and create readable plots with this data. I came to the conclusion, that it is quite difficult to create few informative plots with this large amount of data: we have $4 + n$ values (where $n$ is number of notes being executed) for each of $m$ benchmarks for each of $k$ commits. I can create plots based only on the overall number of cycles for each benchmark, but in that case we will loose valuable data.
I think one of the solutions could be printing both: for example, plots with total number of cycles for each benchmark at the top of the README, and beneath collapsed tabs for each benchmark containing plots for each parameter. But it still looks like a bulky solution for me, IMO it will be difficult to track the performance using that many plots.

So, the general question sounds like: how do we want to show the benchmarking data? Do you know any solutions being able to track and aggregate a lot of data to compare effectiveness? (github-action-benchmark is a good solution, but it doesn't really suit us: it works with the output of actual cargo bench in specific format, and also is able to create plots based on the only one value).

[bobbinth]

I would move the plotting part into a separate issue. For this PR, just committing the JSON file (as you have now), is fine.

[bobbinth]

Looks good! Thank you! I left some small nits inline - once these are addressed, we can merge.

[bobbinth]

All looks good! Thank you! Let's update changelog and merge.

Also, you may want to rebase from the latest next as the changelog was updated there.