Skip to content
This repository has been archived by the owner on Dec 8, 2023. It is now read-only.

Detect contract size regressions #4

Closed
3 tasks
Tracked by #9354
cmichi opened this issue Aug 2, 2021 · 6 comments
Closed
3 tasks
Tracked by #9354

Detect contract size regressions #4

cmichi opened this issue Aug 2, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@cmichi
Copy link
Contributor

cmichi commented Aug 2, 2021

For each CI run we already build each ink! example contract. We should utilize this information to provide insights on how the contract file sizes would change for a particular PR.

ToDo

  • Persist the Wasm size of each compiled contract from master in the artifacts.
  • After each example contract build on a PR compare this size to the size from the artifacts and output the difference. This output should be human-readable, in tabular format.
  • If the contract size got bigger post a comment to the parent PR with the size diffs for all contracts. We don't want to make CI fail if contracts get bigger, since there might be features where the trade-off makes sense. So for now it's just for informational purposes.
@HCastano
Copy link
Contributor

HCastano commented Aug 2, 2021

It's defintely important to track the size of our example contracts over time. However,
I'm not sure if ink-waterfall is the right tool for that.

I think we can do this in the ink! CI already. We already build all the example contracts
as part of the CI, so all we really need is to harvest the cargo-contract output data
and find a way to persist it across runs for analysis purposes.

On a more philosophical note, I view ink-waterfall is a place for testing behaviour,
not properties, so tracking contract size with ink-waterfall doesn't fit into this
model.

@cmichi
Copy link
Contributor Author

cmichi commented Aug 3, 2021

So the idea with including it into ink-waterfall was that the waterfall will eventually be included into the cargo-contract CI as well, hence reducing code duplication. But come to think about it, what might actually be the most elegant way:

  • Create a new repository which contains either a Rust crate or a Bash script which does this regression comparison.
  • Have this crate (or script) be installed in the in the ink! + cargo-contract Docker container.
  • Add a CI stage to both projects which invokes the script with the necessary paths.

Wdyt?

[…] and find a way to persist it across runs for analysis purposes.

This is usually done via GitLab Job Artifacts. Our CI's typically have an anchor reference collect-artifacts which collects everything under artifacts/. You can make this folder available for jobs by requiring the artifacts. So all you have to do is write the results of contract builds on master to this folder, make sure the artifacts are collected and require them in the job.

@cmichi
Copy link
Contributor Author

cmichi commented Aug 3, 2021

A thing that might be tricky is to not post the GitHub comment with the contract sizes again for each CI run, but rather edit the existing comment. Once a comment is posted the first time you likely have to put the comment id into the artifacts as well.

@HCastano
Copy link
Contributor

HCastano commented Aug 3, 2021

I think adding a script to ink!'s scripts folder and using that in a CI stage is the way to go. I'm playing around with that approach in use-ink/ink#885.

Also, question about this:

We should persist the size of just the Wasm blob, as well as the size of the whole .contract bundle (which additionally includes the contract metadata)

Why is keeping track of the bundle size important? Only the Wasm binary is what actually ends up on chain.

@cmichi
Copy link
Contributor Author

cmichi commented Aug 4, 2021

I think adding a script to ink!'s scripts folder and using that in a CI stage is the way to go.

So cargo-contract has a heavy influence over the resulting size as well, hence we should include this regression testing in the CI there as well. If you do it in the way you described you will have to clone the ink! repo in the cargo-contract CI and build each example contract there. If we then subsequently add the ink-waterfall to the cargo-contract CI this then means that all example contracts are built two times, once for your script and once for the waterfall tests. In consequence prolonging the time a CI run takes.

Why is keeping track of the bundle size important? Only the Wasm binary is what actually ends up on chain.

Yes, just limit it to the Wasm, I've adapted the issue.

@HCastano
Copy link
Contributor

HCastano commented Aug 4, 2021

So cargo-contract has a heavy influence over the resulting size as well, hence we should include this regression testing in the CI there as well.

Ah yes, good point. Okay, let me think about this a bit more then

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants