Skip to content

Commit 39da10b

Browse files
Nigel TaoSkia Commit-Bot
authored andcommitted
Optimize SkWuffsCodec pixbuf zero-initialization
Pushing the zero-initialization into the sk_malloc_etc call can be more efficient than a two-step malloc-and-then-memset, e.g. if the memory allocator gives us something from an already-known-zeroed region. Some microbenchmark numbers (milliseconds per decode) from running Chromium's image_decode_bench program on a few GIF files: Before After Ratio Filename Width x Height = Pixels 0.109 0.107 1.02 hat.gif 90 x 112 = 10080 7.062 6.879 1.03 harvesters.gif 1165 x 859 = 1000735 33.342 30.544 1.09 droids.gif 2560 x 1920 = 4915200 Ratio > 1.00 means a performance improvement. image_decode_bench comes from: https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/testing/image_decode_bench.cc hat.gif and harvesters.gif come from: https://github.com/google/wuffs/tree/master/test/data droids.gif comes from: https://cs.chromium.org/chromium/src/third_party/blink/perf_tests/image_decoder/resources/ These are x86_64 Broadwell numbers. I don't have ARM numbers readily available, but nothing about this commit should be arch-specific. Bug: skia:8235 Change-Id: Icf45b7ac5b7194eaab455628e44d4aedf30ddbfe Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254836 Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: Leon Scroggins <scroggo@google.com>
1 parent 301015c commit 39da10b

1 file changed

Lines changed: 26 additions & 9 deletions

File tree

src/codec/SkWuffsCodec.cpp

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ static SkCodec::Result reset_and_decode_image_config(wuffs_gif__decoder* d
132132
return SkCodec::kSuccess;
133133
}
134134

135-
136135
// -------------------------------- Class definitions
137136

138137
class SkWuffsCodec;
@@ -179,6 +178,7 @@ class SkWuffsCodec final : public SkScalingCodec {
179178
std::unique_ptr<SkStream> stream,
180179
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> dec,
181180
std::unique_ptr<uint8_t, decltype(&sk_free)> pixbuf_ptr,
181+
bool pixbuf_zeroed,
182182
std::unique_ptr<uint8_t, decltype(&sk_free)> workbuf_ptr,
183183
size_t workbuf_len,
184184
wuffs_base__image_config imgcfg,
@@ -266,6 +266,10 @@ class SkWuffsCodec final : public SkScalingCodec {
266266
std::vector<SkWuffsFrame> fFrames;
267267
bool fFramesComplete;
268268

269+
// True if fPixelBuffer's contents are known to be already zeroed. This is
270+
// conservative, and may be false even if the buffer is zeroed.
271+
bool fPixbufZeroed;
272+
269273
// If calling an fDecoders[which] method returns an incomplete status, then
270274
// fDecoders[which] is suspended in a coroutine (i.e. waiting on I/O or
271275
// halted on a non-recoverable error). To keep its internal proof-of-safety
@@ -333,6 +337,7 @@ SkWuffsCodec::SkWuffsCodec(SkEncodedInfo&&
333337
std::unique_ptr<SkStream> stream,
334338
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> dec,
335339
std::unique_ptr<uint8_t, decltype(&sk_free)> pixbuf_ptr,
340+
bool pixbuf_zeroed,
336341
std::unique_ptr<uint8_t, decltype(&sk_free)> workbuf_ptr,
337342
size_t workbuf_len,
338343
wuffs_base__image_config imgcfg,
@@ -367,6 +372,7 @@ SkWuffsCodec::SkWuffsCodec(SkEncodedInfo&&
367372
fFrameCountReaderIOPosition(0),
368373
fNumFullyReceivedFrames(0),
369374
fFramesComplete(false),
375+
fPixbufZeroed(pixbuf_zeroed),
370376
fDecoderIsSuspended{
371377
false,
372378
false,
@@ -443,12 +449,20 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
443449
size_t src_bytes_per_pixel = src_bits_per_pixel / 8;
444450

445451
// Zero-initialize Wuffs' buffer covering the frame rect.
446-
wuffs_base__rect_ie_u32 frame_rect = fFrameConfigs[WhichDecoder::kIncrDecode].bounds();
447-
wuffs_base__table_u8 pixels = fPixelBuffer.plane(0);
448-
for (uint32_t y = frame_rect.min_incl_y; y < frame_rect.max_excl_y; y++) {
449-
sk_bzero(pixels.ptr + (y * pixels.stride) + (frame_rect.min_incl_x * src_bytes_per_pixel),
450-
frame_rect.width() * src_bytes_per_pixel);
452+
if (!fPixbufZeroed) {
453+
wuffs_base__rect_ie_u32 frame_rect = fFrameConfigs[WhichDecoder::kIncrDecode].bounds();
454+
wuffs_base__table_u8 pixels = fPixelBuffer.plane(0);
455+
for (uint32_t y = frame_rect.min_incl_y; y < frame_rect.max_excl_y; y++) {
456+
sk_bzero(
457+
pixels.ptr + (y * pixels.stride) + (frame_rect.min_incl_x * src_bytes_per_pixel),
458+
frame_rect.width() * src_bytes_per_pixel);
459+
}
451460
}
461+
// The buffer is zeroed now, but this onStartIncrementalDecode call will
462+
// almost certainly be followed by some onIncrementalDecode calls that can
463+
// modify fPixelBuffer's contents. We set fPixbufZeroed to false so that
464+
// the next onStartIncrementalDecode call will zero-initialize the buffer.
465+
fPixbufZeroed = false;
452466

453467
fIncrDecDst = static_cast<uint8_t*>(dst);
454468
fIncrDecReaderIOPosition = fIOBuffer.reader_io_position();
@@ -886,8 +900,10 @@ std::unique_ptr<SkCodec> SkWuffsCodec_MakeFromStream(std::unique_ptr<SkStream> s
886900
std::unique_ptr<uint8_t, decltype(&sk_free)> workbuf_ptr(
887901
reinterpret_cast<uint8_t*>(workbuf_ptr_raw), &sk_free);
888902

889-
uint64_t pixbuf_len = imgcfg.pixcfg.pixbuf_len();
890-
void* pixbuf_ptr_raw = pixbuf_len <= SIZE_MAX ? sk_malloc_canfail(pixbuf_len) : nullptr;
903+
constexpr int pixbuf_sk_malloc_flags = SK_MALLOC_ZERO_INITIALIZE;
904+
uint64_t pixbuf_len = imgcfg.pixcfg.pixbuf_len();
905+
void* pixbuf_ptr_raw =
906+
pixbuf_len <= SIZE_MAX ? sk_malloc_flags(pixbuf_len, pixbuf_sk_malloc_flags) : nullptr;
891907
if (!pixbuf_ptr_raw) {
892908
*result = SkCodec::kInternalError;
893909
return nullptr;
@@ -919,5 +935,6 @@ std::unique_ptr<SkCodec> SkWuffsCodec_MakeFromStream(std::unique_ptr<SkStream> s
919935
*result = SkCodec::kSuccess;
920936
return std::unique_ptr<SkCodec>(new SkWuffsCodec(
921937
std::move(encodedInfo), std::move(stream), std::move(decoder), std::move(pixbuf_ptr),
922-
std::move(workbuf_ptr), workbuf_len, imgcfg, pixbuf, iobuf));
938+
(pixbuf_sk_malloc_flags & SK_MALLOC_ZERO_INITIALIZE) != 0, std::move(workbuf_ptr),
939+
workbuf_len, imgcfg, pixbuf, iobuf));
923940
}

0 commit comments

Comments
 (0)