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

🧪 Experiment hrmp weighing #351

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented Jul 5, 2024

What?

  • Write hrmp functions benchmarks

Why?

  • They are being called by the xcm executor, so are assigned a fixed weight. We need to make sure we are not going over that

Anything Else?

Next PR will run these benches

Copy link
Contributor Author

JuaniRios commented Jul 5, 2024

@JuaniRios JuaniRios force-pushed the 07-05-experiment_hrmp_weighing branch from e97585b to b0e78d3 Compare July 5, 2024 13:50
@JuaniRios JuaniRios changed the title experiment hrmp weighing 🧪 Experiment hrmp weighing Jul 5, 2024
@JuaniRios JuaniRios force-pushed the 07-05-experiment_hrmp_weighing branch from b0e78d3 to 76410ef Compare July 5, 2024 13:51
@lrazovic lrazovic force-pushed the 07-05-experiment_hrmp_weighing branch from 76410ef to 01b27d8 Compare July 5, 2024 15:31
@JuaniRios JuaniRios mentioned this pull request Jul 5, 2024
@JuaniRios JuaniRios force-pushed the 07-05-experiment_hrmp_weighing branch from 01b27d8 to 8d16a3e Compare July 5, 2024 15:55
@JuaniRios JuaniRios marked this pull request as ready for review July 5, 2024 16:03
Copy link

graphite-app bot commented Jul 5, 2024

Graphite Automations

"Auto-assign PRs to author" took an action on this PR • (07/05/24)

1 assignee was added to this PR based on Juan Ignacio Rios's automation.

@JuaniRios JuaniRios requested review from lrazovic and vstam1 July 5, 2024 16:05
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.

How are you actually going to assign these weights? Or will you just run the weights and hardcode them in the XCM weights config?

@JuaniRios
Copy link
Contributor Author

@vstam1 We are not going to use these weights for now. The struct that computes the XCM weight FixedWeightBounds assigns the same fixed weight to all instructions except Transact, SetErrorHandler, and SetAppendix.
What we are doing in this benchmark is make sure the fixed weight assumption is more than the actual benchmarked weight, which was the case after running the AWS benchmark.

We can add a new case to FixedWeightBounds to use our benchmarked weight for the xcmp notifications, but I don't think it's necessary for now.

Copy link
Collaborator

vstam1 commented Jul 8, 2024

Could we add a test to our runtime that actually checks if these weights are lower then our hardcoded weights?

Copy link
Contributor Author

JuaniRios commented Jul 8, 2024

Merge activity

  • Jul 8, 8:35 AM EDT: @JuaniRios started a stack merge that includes this pull request via Graphite.
  • Jul 8, 8:40 AM EDT: Graphite rebased this pull request as part of a merge.
  • Jul 8, 8:43 AM EDT: @JuaniRios merged this pull request with Graphite.

Base automatically changed from 07-01-benchmarks to main July 8, 2024 12:39
@JuaniRios JuaniRios force-pushed the 07-05-experiment_hrmp_weighing branch from bcebb39 to 804dae0 Compare July 8, 2024 12:40
@JuaniRios JuaniRios merged commit 2f09cf8 into main Jul 8, 2024
@JuaniRios JuaniRios deleted the 07-05-experiment_hrmp_weighing branch July 8, 2024 12:43
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