[rust bmp] Reduce fixed overhead for small decodes This change adds a small memory-backed input path for Rusty BMP. When the `SkStream` already exposes memory and the encoded BMP is small (<=4KB), the codec passes the bytes directly to Rust/image-rs `BmpDecoder` instead of wrapping the stream in `SkStreamAdapter` + `BufReader`. Larger or incremental streams continue to use the existing streaming path. Local Chromium decoder-boundary benchmarks show this reduces small-image Rust overhead while preserving larger-image wins. For example, with the Chromium-side memory-backed stream prep: - 24bit_1x1.bmp: Rust improves from ~6.2 us to ~4.3 us - pal4_1x1.bmp: Rust improves from ~7.4 us to ~5.3 us - bmp-size-32x32-8bpp.bmp remains ~73% faster than legacy C++ - large 32bpp-topdown-320x240.bmp remains ~77% faster than legacy C++ Bug: 452666425 Change-Id: I1d8975f31b7a66b158e8402341399a47dbfe26f4 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1283656 Commit-Queue: Florin Malita <fmalita@google.com> Reviewed-by: Łukasz Anforowicz <lukasza@google.com> Reviewed-by: Florin Malita <fmalita@google.com>
diff --git a/experimental/rust_bmp/decoder/SkBmpRustDecoder.cpp b/experimental/rust_bmp/decoder/SkBmpRustDecoder.cpp index 8f06bab..b3649f2 100644 --- a/experimental/rust_bmp/decoder/SkBmpRustDecoder.cpp +++ b/experimental/rust_bmp/decoder/SkBmpRustDecoder.cpp
@@ -13,7 +13,6 @@ #include "experimental/rust_bmp/decoder/impl/SkBmpRustCodec.h" #include "include/core/SkData.h" #include "include/core/SkStream.h" -#include "src/core/SkStreamPriv.h" namespace SkBmpRustDecoder { @@ -35,7 +34,7 @@ std::unique_ptr<SkCodec> Decode(sk_sp<const SkData> data, SkCodec::Result* result, SkCodecs::DecodeContext) { - return Decode(SkMemoryStream::Make(std::move(data)), result); + return SkBmpRustCodec::MakeFromData(std::move(data), result); } } // namespace SkBmpRustDecoder
diff --git a/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.cpp b/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.cpp index 0fcdd63..af49b30 100644 --- a/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.cpp +++ b/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.cpp
@@ -49,6 +49,8 @@ static_cast<int>(SkEncodedInfo::kUnpremul_Alpha), "BmpAlpha::Unpremul must match SkEncodedInfo::kUnpremul_Alpha"); +static constexpr size_t kInMemoryReaderThreshold = 4 * 1024; + namespace { // Helper function to map Rust DecodingResult to SkCodec::Result @@ -72,6 +74,17 @@ SK_ABORT("Unexpected `rust_bmp::DecodingResult`: %d", static_cast<int>(rustResult)); } +rust::Box<rust_bmp::ResultOfReader> NewReaderFromStream(SkStream* stream) { + auto inputAdapter = std::make_unique<rust::stream::SkStreamAdapter>(stream); + return rust_bmp::new_reader(std::move(inputAdapter)); +} + +rust::Box<rust_bmp::ResultOfReader> NewReaderFromData(const SkData* data) { + rust::Slice<const uint8_t> dataSlice(static_cast<const uint8_t*>(data->data()), + data->size()); + return rust_bmp::new_reader_from_data(dataSlice); +} + } // namespace std::unique_ptr<SkBmpRustCodec> SkBmpRustCodec::MakeFromStream(std::unique_ptr<SkStream> stream, @@ -86,9 +99,40 @@ return nullptr; } - auto inputAdapter = std::make_unique<rust::stream::SkStreamAdapter>(stream.get()); - rust::Box<rust_bmp::ResultOfReader> resultOfReader = - rust_bmp::new_reader(std::move(inputAdapter)); + rust::Box<rust_bmp::ResultOfReader> resultOfReader = NewReaderFromStream(stream.get()); + + return MakeFromStreamAndReader(std::move(stream), std::move(resultOfReader), result, nullptr); +} + +std::unique_ptr<SkBmpRustCodec> SkBmpRustCodec::MakeFromData(sk_sp<const SkData> data, + Result* result) { + Result resultStorage; + if (!result) { + result = &resultStorage; + } + + if (!data) { + *result = kInvalidInput; + return nullptr; + } + + if (!data->empty() && data->size() <= kInMemoryReaderThreshold) { + rust::Box<rust_bmp::ResultOfReader> resultOfReader = NewReaderFromData(data.get()); + std::unique_ptr<SkStream> stream = SkMemoryStream::Make(data); + return MakeFromStreamAndReader(std::move(stream), std::move(resultOfReader), result, + std::move(data)); + } + + return MakeFromStream(SkMemoryStream::Make(std::move(data)), result); +} + +std::unique_ptr<SkBmpRustCodec> SkBmpRustCodec::MakeFromStreamAndReader( + std::unique_ptr<SkStream> stream, + rust::Box<rust_bmp::ResultOfReader> resultOfReader, + Result* result, + sk_sp<const SkData> inMemoryData) { + SkASSERT_RELEASE(stream); + SkASSERT_RELEASE(result); rust_bmp::DecodingResult rustResult = resultOfReader->err(); if (rustResult != rust_bmp::DecodingResult::Success) { @@ -173,19 +217,22 @@ return std::unique_ptr<SkBmpRustCodec>(new SkBmpRustCodec( std::move(encodedInfo), std::move(stream), - std::move(reader) + std::move(reader), + std::move(inMemoryData) )); } SkBmpRustCodec::SkBmpRustCodec(SkEncodedInfo&& encodedInfo, std::unique_ptr<SkStream> stream, - rust::Box<rust_bmp::Reader> reader) + rust::Box<rust_bmp::Reader> reader, + sk_sp<const SkData> inMemoryData) : SkCodec(std::move(encodedInfo), skcms_PixelFormat_RGB_888, // TODO(crbug.com/370522089): Pass stream to SkCodec once SkCodec // avoids unnecessary rewinding (which forces re-reading entire stream). /* stream = */ nullptr) , fReader(std::move(reader)) - , fPrivStream(std::move(stream)) { + , fPrivStream(std::move(stream)) + , fInMemoryData(std::move(inMemoryData)) { SkASSERT_RELEASE(fPrivStream); } @@ -193,6 +240,9 @@ sk_sp<const SkData> SkBmpRustCodec::getEncodedData() const { SkASSERT_RELEASE(fPrivStream); + if (fInMemoryData) { + return fInMemoryData; + } sk_sp<const SkData> data = fPrivStream->getData(); if (data) { return data; @@ -230,9 +280,12 @@ return false; } - auto inputAdapter = std::make_unique<rust::stream::SkStreamAdapter>(fPrivStream.get()); - rust::Box<rust_bmp::ResultOfReader> resultOfReader = - rust_bmp::new_reader(std::move(inputAdapter)); + rust::Box<rust_bmp::ResultOfReader> resultOfReader = [&]() { + if (fInMemoryData) { + return NewReaderFromData(fInMemoryData.get()); + } + return NewReaderFromStream(fPrivStream.get()); + }(); if (resultOfReader->err() != rust_bmp::DecodingResult::Success) { return false;
diff --git a/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.h b/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.h index 1191519..a09a6cf 100644 --- a/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.h +++ b/experimental/rust_bmp/decoder/impl/SkBmpRustCodec.h
@@ -13,10 +13,12 @@ #include "experimental/rust_bmp/ffi/FFI.rs.h" #include "include/codec/SkCodec.h" #include "include/codec/SkEncodedImageFormat.h" +#include "include/core/SkRefCnt.h" #include "include/core/SkSpan.h" #include "third_party/rust/cxx/v1/cxx.h" struct SkEncodedInfo; +class SkData; class SkStream; class SkSwizzler; @@ -28,13 +30,15 @@ class SkBmpRustCodec final : public SkCodec { public: static std::unique_ptr<SkBmpRustCodec> MakeFromStream(std::unique_ptr<SkStream>, Result*); + static std::unique_ptr<SkBmpRustCodec> MakeFromData(sk_sp<const SkData>, Result*); ~SkBmpRustCodec() override; protected: SkBmpRustCodec(SkEncodedInfo&&, std::unique_ptr<SkStream>, - rust::Box<rust_bmp::Reader>); + rust::Box<rust_bmp::Reader>, + sk_sp<const SkData> inMemoryData); SkEncodedImageFormat onGetEncodedFormat() const override { return SkEncodedImageFormat::kBMP; } @@ -43,6 +47,12 @@ bool onGetFrameInfo(int, FrameInfo*) const override; private: + static std::unique_ptr<SkBmpRustCodec> MakeFromStreamAndReader( + std::unique_ptr<SkStream>, + rust::Box<rust_bmp::ResultOfReader>, + Result*, + sk_sp<const SkData>); + // We cannot use the SkCodec implementation since we pass nullptr to the superclass out of // an abundance of caution w/r to rewinding the stream. // @@ -84,6 +94,7 @@ rust::Box<rust_bmp::Reader> fReader; std::unique_ptr<SkStream> fPrivStream; + sk_sp<const SkData> fInMemoryData; std::unique_ptr<SkSwizzler> fSwizzler; // Using 4-bytes-wide `uint32_t` for each pixel, because
diff --git a/experimental/rust_bmp/ffi/FFI.rs b/experimental/rust_bmp/ffi/FFI.rs index fd0ba39..f5aec6d 100644 --- a/experimental/rust_bmp/ffi/FFI.rs +++ b/experimental/rust_bmp/ffi/FFI.rs
@@ -59,6 +59,10 @@ fn new_reader(input: UniquePtr<SkStreamAdapter>) -> Box<ResultOfReader>; + /// Create a Reader from in-memory data, bypassing SkStreamAdapter and + /// BufReader. Use when stream data is fully available in memory. + fn new_reader_from_data(data: &[u8]) -> Box<ResultOfReader>; + type ResultOfReader; fn err(self: &ResultOfReader) -> DecodingResult; fn unwrap(self: &mut ResultOfReader) -> Box<Reader>; @@ -182,9 +186,7 @@ /// BMP decoder with resumable/streaming support. /// Both read_metadata() and read_image_data() can be retried on IncompleteInput. pub struct Reader { - decoder: Option< - image::codecs::bmp::BmpDecoder<std::io::BufReader<cxx::UniquePtr<ffi::SkStreamAdapter>>>, - >, + decoder: Option<image::codecs::bmp::BmpDecoder<DecoderInput>>, metadata_loaded: bool, width: u32, height: u32, @@ -196,19 +198,87 @@ image_data_loaded: bool, last_consumed_row_count: u32, } + +enum DecoderInput { + Stream(std::io::BufReader<cxx::UniquePtr<ffi::SkStreamAdapter>>), + Memory(std::io::Cursor<Vec<u8>>), +} + +impl std::io::Read for DecoderInput { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> { + match self { + DecoderInput::Stream(s) => s.read(buf), + DecoderInput::Memory(c) => c.read(buf), + } + } +} + +impl std::io::BufRead for DecoderInput { + fn fill_buf(&mut self) -> std::io::Result<&[u8]> { + match self { + DecoderInput::Stream(s) => s.fill_buf(), + DecoderInput::Memory(c) => c.fill_buf(), + } + } + fn consume(&mut self, amt: usize) { + match self { + DecoderInput::Stream(s) => s.consume(amt), + DecoderInput::Memory(c) => c.consume(amt), + } + } +} + +impl std::io::Seek for DecoderInput { + fn seek(&mut self, pos: std::io::SeekFrom) -> std::io::Result<u64> { + match self { + DecoderInput::Stream(s) => s.seek(pos), + DecoderInput::Memory(c) => c.seek(pos), + } + } + + fn stream_position(&mut self) -> std::io::Result<u64> { + match self { + DecoderInput::Stream(s) => s.stream_position(), + DecoderInput::Memory(c) => c.stream_position(), + } + } +} + impl Reader { fn new(input: cxx::UniquePtr<ffi::SkStreamAdapter>) -> Result<Self, BmpError> { use std::io::BufReader; - // Use a larger buffer (64KB) to reduce syscall overhead for large images. - // Default BufReader uses 8KB which causes many small reads. + // Keep the stream fallback at the previous size to avoid regressing + // large or incremental decodes. Complete small images bypass this path + // via new_reader_from_data(). const BUFFER_SIZE: usize = 64 * 1024; let buffered = BufReader::with_capacity(BUFFER_SIZE, input); + let input = DecoderInput::Stream(buffered); // new_resumable() creates the decoder without reading any data from the stream. // The caller must call read_metadata() to read the BMP headers. // This constructor is infallible - it just wraps the reader. - let decoder = image::codecs::bmp::BmpDecoder::new_resumable(buffered); + let decoder = image::codecs::bmp::BmpDecoder::new_resumable(input); + + Ok(Reader { + decoder: Some(decoder), + metadata_loaded: false, + width: 0, + height: 0, + color: BmpColor::RGB, + alpha: BmpAlpha::Opaque, + bytes_per_pixel: 0, + image_data: Vec::new(), + image_data_loaded: false, + last_consumed_row_count: 0, + }) + } + + /// Create a Reader from in-memory data. The bytes are copied into + /// Rust-owned storage because Reader outlives this FFI call. + fn from_data(data: &[u8]) -> Result<Self, BmpError> { + let input = DecoderInput::Memory(std::io::Cursor::new(data.to_vec())); + let decoder = image::codecs::bmp::BmpDecoder::new_resumable(input); Ok(Reader { decoder: Some(decoder), @@ -304,11 +374,12 @@ None => return DecodingResult::FormatError, }; - // Allocate zero-initialized buffer for the image data. - // Zero-initialization ensures that any unwritten bytes (e.g., on partial - // decode) are valid zeros rather than garbage. + // Reuse existing allocation if capacity is sufficient (e.g., after + // reset_decode_state which clears length but preserves capacity). + // This avoids a fresh heap allocation on rewind/re-decode. if self.image_data.len() < total_bytes { - self.image_data = vec![0u8; total_bytes]; + self.image_data.clear(); + self.image_data.resize(total_bytes, 0); } match decoder.read_image_data(&mut self.image_data) { @@ -415,6 +486,11 @@ Box::new(ResultOfReader { result }) } +pub fn new_reader_from_data(data: &[u8]) -> Box<ResultOfReader> { + let result = Reader::from_data(data); + Box::new(ResultOfReader { result }) +} + #[cfg(test)] mod tests { use super::*;