-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[benchmark] Add option for package overrides #15841
base: main
Are you sure you want to change the base?
Conversation
731260e
to
c65a196
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great and a very useful functionality. Left a few minor comments.
long, | ||
num_args = 1.., | ||
value_delimiter = ' ', | ||
help = "List of space-separated paths to compiled / built packages with Move code" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another cool feature this brings (and I think we should be explicit about it in the readme) is that you can compile using different compiler versions/optimization levels/experimental compiler stuff and just point to the built packages, giving the ability to compare them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes yes yes! Exactly, but for that I need to modify CLI to pass flags to build option, or instead get already build packages from the path (ideally)
// 1. Override gas schedule, to track the costs of charging gas or tracking limits. | ||
// 2. BlockExecutorConfigFromOnchain to experiment with different block cutting based | ||
// on gas limits?. | ||
// 3. Build options for package overrides. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so this would allow pointing to just source. Perhaps mention that right now, the build option is only used for getting the bytecode version (and that this wont recompile the package with move-2).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now compilation happens with hardcoded options, ideally one should be able to paramterize this, do you have specific things in mind I should add? e.g., how do I enable optimization levels, etc.
My original plan was to point to already build package (so that using compiler is external), but I did not find a way to retrieve the package from the filesystem (i.e., we don't have APIs for that, etc.) so instead just used this for now. You can probably suggest more here from compiler side?
c65a196
to
103467a
Compare
103467a
to
a8fc584
Compare
Description
This PR adds support for package overriding: one can specify
--override-packages P1 P2 P3
and replay historical transactions with new code (e.g., framework). This allows to check performance (benchmark
option) or gas usage (diff
option).Note: one needs to be careful overriding packages which add new package dependencies. For example, it is possible that stdlib adds a module, which is used in framework code. For override, one needs to provide both packages.
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist