Skip to content
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

🛠️ Rework benchmarks #374

Merged
merged 1 commit into from
Aug 23, 2024
Merged

🛠️ Rework benchmarks #374

merged 1 commit into from
Aug 23, 2024

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented Aug 7, 2024

What?

  • Fix and modify pallet-funding benchmarks post-simplification

Why?

  • Our benchmarks were overtly complicated, a result of the old logic.

How?

  • Fix syntax for new instantiator functions
  • Simplify setup
  • Delete old logic branches (e.g. no longer "unchanged" outcome for evaluator, no more benchmark for that)
  • When 2 logic branches only had a small difference (e.g. 1 storage read/write), we now benchmark the worst case.

Testing?

cargo test --features runtime-benchmarks
just dry-run-benchmarks
Keep in mind you need to do a cargo clean for it to work (dunno why)

Anything Else?

Had to delete the runtime weights since the weight struct looks different. The runtime now uses the default () from the new weight file I generated in the pallet.

Soon we will generate the real pallet/runtime benchmarks in AWS

@JuaniRios JuaniRios changed the title Fix benchmarks 🛠️ Fix benchmarks Aug 7, 2024
@JuaniRios JuaniRios self-assigned this Aug 7, 2024
@JuaniRios JuaniRios force-pushed the 08-07-fix_benchmarks branch 5 times, most recently from 96311f9 to 3ba99a9 Compare August 8, 2024 12:08
@JuaniRios JuaniRios force-pushed the 08-07-fix_benchmarks branch 2 times, most recently from a02d194 to 70b4f51 Compare August 8, 2024 14:18
@JuaniRios JuaniRios marked this pull request as ready for review August 8, 2024 15:19
@JuaniRios JuaniRios changed the title 🛠️ Fix benchmarks 🛠️ Rework benchmarks Aug 9, 2024
@JuaniRios JuaniRios changed the base branch from 08-06-new_tests to 08-09-remove_auctioninitializeperiod August 9, 2024 11:36
@JuaniRios JuaniRios force-pushed the 08-09-remove_auctioninitializeperiod branch from 275751b to 8def483 Compare August 9, 2024 11:41
@JuaniRios JuaniRios force-pushed the 08-07-fix_benchmarks branch 2 times, most recently from 3786682 to 4215056 Compare August 9, 2024 11:51
@JuaniRios JuaniRios force-pushed the 08-09-remove_auctioninitializeperiod branch from 8def483 to aa77143 Compare August 13, 2024 09:04
@JuaniRios JuaniRios force-pushed the 08-09-remove_auctioninitializeperiod branch from aa77143 to deac748 Compare August 13, 2024 09:27
@JuaniRios JuaniRios force-pushed the 08-09-remove_auctioninitializeperiod branch from deac748 to 44ce271 Compare August 13, 2024 09:39
This was referenced Aug 13, 2024
@lrazovic lrazovic force-pushed the 08-09-remove_auctioninitializeperiod branch from 44ce271 to 7b4f8cd Compare August 16, 2024 13:04
@lrazovic lrazovic mentioned this pull request Aug 16, 2024
@JuaniRios JuaniRios force-pushed the 08-09-remove_auctioninitializeperiod branch from 7b4f8cd to 44ce271 Compare August 16, 2024 13:10
@lrazovic lrazovic force-pushed the 08-09-remove_auctioninitializeperiod branch from 44ce271 to 7b4f8cd Compare August 16, 2024 13:14
Copy link
Collaborator

@vstam1 vstam1 left a comment

Choose a reason for hiding this comment

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

Love to see a net code removal of 1300 lines in the benchmarks! Left a couple of comments

pallets/funding/src/benchmarking.rs Show resolved Hide resolved
runtimes/polimec/src/lib.rs Show resolved Hide resolved
runtimes/polimec/src/lib.rs Show resolved Hide resolved
Copy link
Contributor Author

JuaniRios commented Aug 23, 2024

Merge activity

  • Aug 23, 5:56 AM EDT: @JuaniRios started a stack merge that includes this pull request via Graphite.
  • Aug 23, 6:47 AM EDT: Graphite rebased this pull request as part of a merge.
  • Aug 23, 6:48 AM EDT: @JuaniRios merged this pull request with Graphite.

@JuaniRios JuaniRios changed the base branch from 08-09-remove_auctioninitializeperiod to graphite-base/374 August 23, 2024 10:41
@JuaniRios JuaniRios changed the base branch from graphite-base/374 to main August 23, 2024 10:43
@JuaniRios JuaniRios merged commit deeccca into main Aug 23, 2024
1 check passed
@JuaniRios JuaniRios deleted the 08-07-fix_benchmarks branch August 23, 2024 10:48
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