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

fix(compute/build): normalise and bucket heap allocations #1130

Merged
merged 4 commits into from Feb 14, 2024

Conversation

Integralist
Copy link
Collaborator

@Integralist Integralist commented Feb 14, 2024

Problem

We started annotating Wasm binaries with additional metadata and part of that data was the number of memory heap allocations. Including this metric means every time you build a package (even if you have changed ZERO lines of code) it'll produce a package with a different hash (e.g. fastly compute hash-files). This behaviour breaks Terraform's ability to reason about whether the package has changed, as each time you build the amount of memory used to create the package is variable/fluctuating.

Solution

We're going to bucket the memory allocation numbers to provide a consistent output. If the memory allocations increase significantly then the bucket value will change (as we're considering the change significant enough to be considered some kind of important code/dependency change).

Screenshots

Below demonstrates me building a package twice (no code changes) and the memory allocation value is now consistent (2-5MB):

Screenshot 2024-02-14 at 17 07 53

The following demonstrates me building a package twice (no code changes) and the hash produced is thus the same:

Screenshot 2024-02-14 at 17 23 27

@Integralist Integralist added the bug Something isn't working label Feb 14, 2024
@Integralist Integralist changed the title Integralist/fix annotations fix(compute/build): normalise and bucket heap allocations Feb 14, 2024
pkg/commands/compute/build.go Outdated Show resolved Hide resolved
Co-authored-by: Cameron Walters (cee-dub) <cameron.walters@gmail.com>
@Integralist
Copy link
Collaborator Author

Thanks @cee-dub 👍🏻

@Integralist Integralist merged commit 9f9b913 into main Feb 14, 2024
8 checks passed
@Integralist Integralist deleted the integralist/fix-annotations branch February 14, 2024 17:58
jdno added a commit to jdno/rust-simpleinfra that referenced this pull request Feb 15, 2024
The collection of metadata was disabled in rust-lang#387, because the metadata
contained an unstable property that broke the change detection in
Terraform. This issue has been resolved upstream by introducing buckets
for the heap allocation size, which reduces the risk of changes in the
metadata while preserving the usefulness of the property.

See fastly/cli#1130 for details on the buckets.
jdno added a commit to jdno/rust-simpleinfra that referenced this pull request Apr 16, 2024
The collection of metadata was disabled in rust-lang#387, because the metadata
contained an unstable property that broke the change detection in
Terraform. This issue has been resolved upstream by introducing buckets
for the heap allocation size, which reduces the risk of changes in the
metadata while preserving the usefulness of the property.

See fastly/cli#1130 for details on the buckets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants