Update zerocopy to v0.8
#384
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This branch updates our `zerocopy` dependency from v0.6.x to v0.8.x. I initially made the upgrade as I wanted to use some new `zerocopy` features in #370, but have factored it out to land separately. Of course, we'll need to update Hubris and MGS, as well. Note that `zerocopy`'s `read_from_prefix` now returns the rest of the buffer, making some code a little bit simpler where we were previously doing that manually. Other than that, there's not a lot to this change, besides deriving some additional marker traits (`Immutable` and `KnownLayout`).
mkeeter
approved these changes
Apr 29, 2025
jgallagher
approved these changes
Apr 30, 2025
|
I had held off on merging this until I could check that Hubris actually builds against it, but after trying it out in oxidecomputer/hubris#2054, I think this branch is good to go. |
hawkw
added a commit
to oxidecomputer/hubris
that referenced
this pull request
May 2, 2025
now that oxidecomputer/management-gateway-service#384 has merged.
hawkw
added a commit
to oxidecomputer/hubris
that referenced
this pull request
May 9, 2025
This branch updates our dependency on `zerocopy` from v0.6.x to v0.8.x. The primary motivation for this change is that I had wanted to use `zerocopy` v0.8's support for data-bearing `enum`s in the `gateway-ereport-messages` crate I added in oxidecomputer/management-gateway-service#370, and...I hadn't realized how painful taking the `zerocopy` update in Hubris would be. :) But, it's also just good to stay on top of new dependency versions regardless. This is a _very_ large change, since pretty much every place where we derive or use `zerocopy`'s traits needed to be changed slightly to use the new APIs, but for the most part, it's not actually that *interesting*, so reviewing it should be pertty straightforward. The main API changes that are worth noting are: - `AsBytes` is now called `IntoBytes`, which was an easy update. - All the `_from_prefix`/`_from_suffix` APIs now return the rest of the slice, which is a nice improvement --- previously we would manually split off the rest of the slice when using those functions. - Conversions from bytes now return `Result`s instead of `Option`s, which very trivial changes in a few places. - `LayoutVerified` is replaced by a new `Ref` type, but other API changes mean that you now basically never need to use it, which rocks! - Some methods were renamed, which was also a pretty trivial find-and-replace. - `zerocopy` adds new `Immutable` and `KnownLayout` marker traits. `Immutable` is required for `IntoBytes::as_bytes()`; types which do not derive it are now assumed to have interior mutability and only provide `IntoBytes::as_bytes_mut()`. So, basically everything now needs to derive `Immutable`. `KnownLayout` isn't required for APIs we use as commonly but I added it on everything anyway. This was most of what made this update annoying. - `FromBytes::new_zeroed` is now provided by a new `FromZeros` trait, but the `FromBytes` derive implements that trait as well. There may be some places where we can now make better use of new `zerocopy` features. In particular, we can now use `zerocopy` on data-bearing enums, which might allow us to replace `hubpack` with `zerocopy` in several places. I didn't do that in this PR, but it's worth looking into in follow-up changes --- I just wanted to get everything building with the new API and felt that improving our usage of it would be better off done in smaller commits. One other important thing to note is that updating the `gateway-messages` dependency increased stack depth in `control-plane-agent` from 6000B to 6136B for non-sidecar targets, so I bumped them up to 6256B. This, in turn, increases RAM to 65525B for `control-plane-agent`, which exceeds the `max-sizes` config, per @cbiffle's previous advice, I just deleted all the `max-sizes` for `control-plane-agent` tasks. Furthermore, this branch requires the following changes to other crates to pick up the latest `zerocopy`: - oxidecomputer/management-gateway-service#384 - oxidecomputer/idolatry#57 - oxidecomputer/humpty#8 Those ought to merge first so we can point our Git dependencies on those repos back at the `main` branch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch updates our
zerocopydependency from v0.6.x to v0.8.x. I initially made the upgrade as I wanted to use some newzerocopyfeatures in #370, but have factored it out to land separately. Of course, we'll need to update Hubris and MGS, as well.Note that
zerocopy'sread_from_prefixnow returns the rest of the buffer, making some code a little bit simpler where we were previously doing that manually. Other than that, there's not a lot to this change, besides deriving some additional marker traits (ImmutableandKnownLayout).