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.

Sign up for GitHub

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

Jump to bottom

libcnb-test: Support compiling buildpacks with --release #386

Closed
edmorley opened this issue Mar 29, 2022 · 2 comments · Fixed by #456
Closed

libcnb-test: Support compiling buildpacks with --release #386

edmorley opened this issue Mar 29, 2022 · 2 comments · Fixed by #456
Assignees
@edmorley
Labels
enhancement New feature or request libcnb-test

Comments

@edmorley
Copy link
Member

edmorley commented Mar 29, 2022 •
edited
Loading

In GUS-W-10910805 / rust-lang/flate2-rs/issues/297, I found that when using flate2 to decompress a downloaded file (the Python runtime archive), debug builds are so slow as to make them unusable for development iteration/testing, particularly when combined with Docker's QEMU performance issues on M1. (See the table of runtimes in that issue for more details.)

For manual testing using cargo libcnb package followed by pack build, I can work around this by passing --release to cargo libcnb package, however currently libcnb-test does not provide a way to request that the buildpack is compiled in release mode.

In addition to adding support for this being opt in, we should evaluate whether running in release mode should be the default (depends on where the break-even point is for compile time vs runtime, though effective target/ caching can help with the former).

GUS-W-10912497.
@Malax
Copy link
Member

Malax commented Mar 29, 2022

The linked issue also has a comment with an IMO sensible workaround: bumping the optimisation level for debug up a notch in Cargo.toml:

[profile.dev]
opt-level = 1

@edmorley
Copy link
Member Author

Yes I'm aware. I still think we should support this however.

@edmorley edmorley self-assigned this Jul 6, 2022
@edmorley edmorley added the enhancement New feature or request label Jul 6, 2022
@edmorley edmorley mentioned this issue Jul 6, 2022
Merged
edmorley added a commit that referenced this issue Jul 6, 2022
@edmorley
Support testing buildpacks compiled in release mode
d0c0c46 
Previously `libcnb-test` only supported compiling the buildpack
under test using Cargo's `dev` profile.

Now, `TestConfig::cargo_profile` can be used along with `CargoProfile`
to instruct `libcnb-package` to compile the buildpack using Cargo's
`release` mode (equivalent to `--release`).

The `TestConfig` default profile remains `CargoProfile::Dev` as before.

This:
- Allows buildpack maintainers to opt into release mode, if their
  buildpack's tests happen to run particularly slowly in debug mode.
  Whilst release builds take longer, in some cases this is vastly offset
  by the faster runtime performance of release builds (particularly when
  the compilation cache is warm).
- Could help in the future should we encounter any "release mode only"
  compiler related bugs, where being able to easily run integration tests
  in release mode would be invaluable.

An integration test has not been added, since:
- There is no easy way to determine in the test whether the buildpack was
  actually built in release mode or not (we don't expose the Cargo logs).
- Most of this change has coverage via the type system.
- The release mode is not new, and is already exercised via `libcnb-cargo`.

Fixes #386.
GUS-W-10912497.
edmorley added a commit that referenced this issue Jul 6, 2022
@edmorley
Support testing buildpacks compiled in release mode (#456)
2550475 
Previously `libcnb-test` only supported compiling the buildpack
under test using Cargo's `dev` profile.

Now, `TestConfig::cargo_profile` can be used along with `CargoProfile`
to instruct `libcnb-package` to compile the buildpack using Cargo's
`release` mode (equivalent to `--release`).

The `TestConfig` default profile remains `CargoProfile::Dev` as before.

This:
- Allows buildpack maintainers to opt into release mode, if their
  buildpack's tests happen to run particularly slowly in debug mode.
  Whilst release builds take longer, in some cases this is vastly offset
  by the faster runtime performance of release builds (particularly when
  the compilation cache is warm).
- Could help in the future should we encounter any "release mode only"
  compiler related bugs, where being able to easily run integration tests
  in release mode would be invaluable.

An integration test has not been added, since:
- There is no easy way to determine in the test whether the buildpack was
  actually built in release mode or not (we don't expose the Cargo logs).
- The integration test would be the only one in this repo to run in release mode,
  so would add to the end to end time due to it having a cold compilation cache,
  and also bloat the CI cache (due to there being a mix of debug+release builds;
  something that people using release mode long-term in their own buildpack
  are unlikely to do).
- Most of this change has coverage via the type system +
  `TestConfig::cargo_profile` rustdoc example.
- The release mode is not new, and is already exercised via `libcnb-cargo`.

Fixes #386.
GUS-W-10912497.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@edmorley edmorley

Labels
enhancement New feature or request libcnb-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support testing buildpacks compiled in release mode heroku/libcnb.rs
2 participants
@Malax @edmorley