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

Investigate if caching actually helps & how much #24

Closed
dmikusa opened this issue Mar 25, 2021 · 2 comments · Fixed by #41
Closed

Investigate if caching actually helps & how much #24

dmikusa opened this issue Mar 25, 2021 · 2 comments · Fixed by #41

Comments

@dmikusa
Copy link
Contributor

dmikusa commented Mar 25, 2021

Right now the buildpack will look at the Cargo.lock file, take its hash, and store that for future runs. The next time it runs, the buildpack will pull this hash & compare it to the hash of the Cargo.lock file that was pushed. The assumption is that if the two are different, then we need to rebuild. That seems likely to be true, but I'm not sure if that is always true and I think if it will hold true it will depend on the cargo install arguments used. Given that we'll be allowing users to set their own custom arguments in #19, this might become a problem.

At the same time, it's not clear that this is really doing anything advantageous. If we are saving state between builds, which we try to do (the target/ directory is placed into its own layer marked with build + cache), cargo install should be able to run and make its own determination if something needs to be rebuilt. This should still be fast, but should also be accurate.

After #19 is merged, we need to investigate if for small/medium/large projects taking out this hash-based caching impacts the build speed & if so, how much. If the impact is minimal, as expected, then we can take out this caching.

@dmikusa
Copy link
Contributor Author

dmikusa commented Apr 4, 2021

Did a little more playing with this and it needs to be removed. There are cases where it causes re-builds to not happen.

@dmikusa
Copy link
Contributor Author

dmikusa commented Apr 4, 2021

Did a little more playing with this and it definitely impacts performance. It basically makes everything 100% re-build on every pack build, which is very bad. I'm pretty sure this is because CARGO_HOME is not being set (see #25). That's where downloads are stored & it's not being cached now.

dmikusa pushed a commit that referenced this issue Apr 18, 2021
- Remove buildpack's layer caching. Caching for the layer could be difficult, so removing it just leaves things up to Cargo which is the most accurate
- Creates two folders under the cargo cache layer, `target/` and `home/`. The former is where build files go while the latter is where `cargo` cached downloads, like from crates.io go.
- At the moment, a layer cached by the lifecycle does not preserve file mtimes. They are squashed in the name of reproducible builds. To get around this, the buildpack will preserve the mtimes of everything install, after cargo runs & then restore them the next time before cargo runs. This keeps consistent file mtimes, which is necessary for cargo to work properly.
- Removes unnecessary directories from cargo's home directory, based on https://doc.rust-lang.org/cargo/guide/cargo-home.html#caching-the-cargo-home-in-ci, which saves space on the cache layer.

Resolves: #24 and #25
ForestEckhardt pushed a commit that referenced this issue Apr 21, 2021
- Remove buildpack's layer caching. Caching for the layer could be difficult, so removing it just leaves things up to Cargo which is the most accurate
- Creates two folders under the cargo cache layer, `target/` and `home/`. The former is where build files go while the latter is where `cargo` cached downloads, like from crates.io go.
- At the moment, a layer cached by the lifecycle does not preserve file mtimes. They are squashed in the name of reproducible builds. To get around this, the buildpack will preserve the mtimes of everything install, after cargo runs & then restore them the next time before cargo runs. This keeps consistent file mtimes, which is necessary for cargo to work properly.
- Removes unnecessary directories from cargo's home directory, based on https://doc.rust-lang.org/cargo/guide/cargo-home.html#caching-the-cargo-home-in-ci, which saves space on the cache layer.

Resolves: #24 and #25
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 a pull request may close this issue.

1 participant