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

feat: add xcm benchmarking #995

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

feat: add xcm benchmarking #995

wants to merge 35 commits into from

Conversation

dmoka
Copy link
Contributor

@dmoka dmoka commented Jan 22, 2025

#989

TODO:

  • double check if everything is cheaper. If everything is chaper, we wait 2409, if not we are good to go
  • UI should test it out and change dest fees by running chopstick matrix
  • talk to moonbeam about this changes
  • regenerate benches on ref machine
  • fix integration tests

@dmoka dmoka linked an issue Jan 22, 2025 that may be closed by this pull request
@dmoka dmoka self-assigned this Jan 22, 2025
@dmoka dmoka marked this pull request as ready for review January 22, 2025 16:30
@dmoka dmoka marked this pull request as draft January 22, 2025 19:47
@dmoka dmoka marked this pull request as ready for review January 23, 2025 09:02
Copy link

github-actions bot commented Jan 23, 2025

Crate versions that have been updated:

  • runtime-integration-tests: v1.32.1 -> v1.33.0
  • hydradx-runtime: v284.0.0 -> v285.0.0

Runtime version has been increased.

@dmoka dmoka requested review from jak-pan and Roznovjak January 23, 2025 12:32
@jak-pan
Copy link
Contributor

jak-pan commented Feb 7, 2025

How did you check the weights are lower than b4? Can you drop old bench results somewhere so we can confirm?

fun: Fungible(ExistentialDeposit::get()),
}
}
}

use primitives::constants::currency::UNITS;
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably split this to separate file as it's getting crowded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you remember, the last time when we tried to put the runtime apis into a separate file, we had a weird error on prod, so maybe it is just better to leave it in lib.rs...

@dmoka
Copy link
Contributor Author

dmoka commented Feb 7, 2025

How did you check the weights are lower than b4? Can you drop old bench results somewhere so we can confirm?

Just did some testing. Not all extrinsic became cheaper.

Here is the old weight for all xcm extrinsic:

	pub const BaseXcmWeight: XcmWeight = XcmWeight::from_parts(100_000_000, 0);

This was used as an "overestimate". It seems it was not that good overestimate.

Here are the xcm extrinsics that became more expensive after the current rebenchmarking:

	withdraw_asset 89.25
	transfer_asset 204.92
	transfer_reserve_asset 583%
	initiate_reserve_withdraw 326.35%
	receive_teleported_asset  18446744073609.55% (Weight::MAX as not used)
	deposit_asset  57.39%
	deposit_reserve_asset   455.64%
	report_holding 318.42%
	report_error 280.14%
	claim_asset 39.10%
	subscribe_version 382.13%
	unsubscribe_version  4.51%
	query_pallet 295.17%
	report_transact_status   280.23%
	exchange_asset:  3848.85%

And here are the ones that became cheaper:

	reserve_asset_deposited 98.01%
	buy_execution  98.71%
	query_response 65.93%
	transact 89.38%
	refund_surplus 95.54%
	set_error_handler 98.76%
	set_appendix 98.78%
	clear_error 98.79%
	descend_origin 98.76%
	clear_origin 98.81%
	trap 98.83%
	burn_asset 74.84%
	expect_asset 92.23%
	expect_origin 98.82%
	expect_error 98.80%
	expect_transact_status 98.57%
	expect_pallet 86.65%
	clear_transact_status 98.73%
	set_topic 98.80%
	clear_topic 98.80%
	unpaid_execution  98.77%
	set_fees_mode 98.82%

Based on this result, it seems we gotta wait for 2409 to not to break any UI, that's what we discussed last time?

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.

Benchmark XCM weights in a better way
2 participants