imageproxy: make ImageProxy: Send + Sync #78
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
The internal detail of how we handle the exit of the child process leads to the entire ImageProxy struct not being Send, which means that it can't be used in cross-thread Futures with tokio. Fortunately it's a very simple fix. Add some trivial static testing to make sure this doesn't regress. Add it for OpenedImage as well: it's currently trivially Send+Sync on account of being a wrapper around an integer, but this might change in the future. This change has been cross-checked against both bootc (ostree-ext) and composefs-rs and doesn't cause any issues. Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya
force-pushed
the
send
branch
from
5ebc540 to
560d344
Compare
allisonkarlitskaya
added a commit
to allisonkarlitskaya/composefs-rs
that referenced
this pull request
Apr 22, 2025
Add an async version of `Repository::ensure_object()` and wire it through `SplitStreamWriter::write_external()`. Call that when we're splitting OCI layer tarballs to offload the writing of external objects (and the `fdatasync()` that goes with it) to a separate thread. This is something like some prep work for something we've been trying to accomplish for a while in containers#62 but it doesn't come close to the complete picture (since it still writes the objects sequentially). Modify the (already) async code in oci::ImageOp to download layers in parallel. This is a big deal for images with many layers (as is often the case for bootc images, due to the splitting heuristics). This takes a pull of the Fedora Silverblue 42 container image (when pulled from a local `oci-dir`) from ~90s to ~8.5s time to complete on my laptop. Unfortunately, container images made from large single layers are not substantially improved. In order to make this change we need to depend on a pre-release version of containers-image-proxy-rs which makes ImageProxy: Send + Sync. See bootc-dev/containers-image-proxy-rs#78 for that. This prevents us from publishing ourselves for now, so make our check for that non-fatal. Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya
added a commit
to allisonkarlitskaya/composefs-rs
that referenced
this pull request
Apr 22, 2025
Add an async version of `Repository::ensure_object()` and wire it through `SplitStreamWriter::write_external()`. Call that when we're splitting OCI layer tarballs to offload the writing of external objects (and the `fdatasync()` that goes with it) to a separate thread. This is something like some prep work for something we've been trying to accomplish for a while in containers#62 but it doesn't come close to the complete picture (since it still writes the objects sequentially). Modify the (already) async code in oci::ImageOp to download layers in parallel. This is a big deal for images with many layers (as is often the case for bootc images, due to the splitting heuristics). This takes a pull of the Fedora Silverblue 42 container image (when pulled from a local `oci-dir`) from ~90s to ~8.5s time to complete on my laptop. Unfortunately, container images made from large single layers are not substantially improved. In order to make this change we need to depend on a pre-release version of containers-image-proxy-rs which makes ImageProxy: Send + Sync. See bootc-dev/containers-image-proxy-rs#78 for that. This prevents us from publishing ourselves for now, so make our check for that non-fatal. Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya
added a commit
to allisonkarlitskaya/composefs-rs
that referenced
this pull request
Apr 22, 2025
Add an async version of `Repository::ensure_object()` and wire it through `SplitStreamWriter::write_external()`. Call that when we're splitting OCI layer tarballs to offload the writing of external objects (and the `fdatasync()` that goes with it) to a separate thread. This is something like some prep work for something we've been trying to accomplish for a while in containers#62 but it doesn't come close to the complete picture (since it still writes the objects sequentially). Modify the (already) async code in oci::ImageOp to download layers in parallel. This is a big deal for images with many layers (as is often the case for bootc images, due to the splitting heuristics). This takes a pull of the Fedora Silverblue 42 container image (when pulled from a local `oci-dir`) from ~90s to ~8.5s time to complete on my laptop. Unfortunately, container images made from large single layers are not substantially improved. In order to make this change we need to depend on a pre-release version of containers-image-proxy-rs which makes ImageProxy: Send + Sync. See bootc-dev/containers-image-proxy-rs#78 for that. This prevents us from publishing ourselves for now, so make our check for that non-fatal. Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
|
Also related #31 |
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.
The internal detail of how we handle the exit of the child process leads to the entire ImageProxy struct not being Send, which means that it can't be used in cross-thread Futures with tokio. Fortunately it's a very simple fix.
Add some trivial static testing to make sure this doesn't regress. Add it for OpenedImage as well: it's currently trivially Send+Sync on account of being a wrapper around an integer, but this might change in the future.
This change has been cross-checked against both bootc (ostree-ext) and composefs-rs and doesn't cause any issues.