wuffs 0.4 significantly slower than 0.3 decoding PNGs

[Ono-Sendai]

On Windows, Visual studio 2022, no AVX, AMD Ryzen 9 5950x.

wuffs 0.3, decoding to WUFFS_BASE__PIXEL_FORMAT__RGB
-----------------------------------------------------
doDecodeFromBufferWithWuffs took 8.4251 ms
doDecodeFromBufferWithWuffs took 8.3683 ms
doDecodeFromBufferWithWuffs took 8.4120 ms

wuffs 0.4, decoding to WUFFS_BASE__PIXEL_FORMAT__RGB
-----------------------------------------------------
doDecodeFromBufferWithWuffs took 12.893 ms
doDecodeFromBufferWithWuffs took 12.906 ms
doDecodeFromBufferWithWuffs took 12.829 ms

[nigeltao]

Thanks for the bug report. I don't run Windows or VS myself. Are you able to bisect a few Wuffs versions to see which commit caused the slow down?

If you're using wuffs-v0.4.c then there's only a handful of relevant commits (and f1698226 is the most recent):

f1698226 (tag: v0.4.0-alpha.4) release: wuffs gen -version=0.4.0-alpha.4
5c693c9d (tag: v0.4.0-alpha.3) release: wuffs gen -version=0.4.0-alpha.3
3eb4e3a6 (tag: v0.4.0-alpha.2) release: wuffs gen -version=0.4.0-alpha.2
6381a325 (tag: v0.4.0-alpha.1) release: wuffs gen -version=0.4.0-alpha.1

That's pretty coarse-grained though. If you have the time, I'd find it more helpful if you can bisect using wuffs-unsupported-snapshot.c instead. The two endpoints (below) are about 343 commits apart, so a bisect should take 9 cuts.

f1698226 (tag: v0.4.0-alpha.4) release: wuffs gen -version=0.4.0-alpha.4
a27a7598 base: disable SIMD on 32-bit (not 64-bit) x86
...
9c03b8c7 wuffsfmt: double-indents hanging lines
d910658b (tag: v0.3.3) wuffs gen -version=0.3.3

---

Also, what compiler flags are you using?

[nigeltao]

Tangentially, if this is your code:

https://github.com/glaretechnologies/glare-core/blob/2c7174ca95c68bfc4cea982ec12ac60db0bfd61d/graphics/PNGDecoder.cpp#L276

https://github.com/glaretechnologies/glare-core/blob/2c7174ca95c68bfc4cea982ec12ac60db0bfd61d/graphics/PNGDecoder.cpp#L322

Then an explicit "Convert from BGR to RGB" shouldn't be necessary, Instead, I think that you can change line 276 from this:

use_pixelformat = WUFFS_BASE__PIXEL_FORMAT__BGR

to this:

use_pixelformat = WUFFS_BASE__PIXEL_FORMAT__RGB

[nigeltao]

I'd also like to know whether any or these #ifdefs trigger for your VS compiler flags:

#if defined(_M_IX86)
#if defined(_M_X64)
#if defined(__AVX__)

[Ono-Sendai]

Tangentially, if this is your code:

https://github.com/glaretechnologies/glare-core/blob/2c7174ca95c68bfc4cea982ec12ac60db0bfd61d/graphics/PNGDecoder.cpp#L276

https://github.com/glaretechnologies/glare-core/blob/2c7174ca95c68bfc4cea982ec12ac60db0bfd61d/graphics/PNGDecoder.cpp#L322

Then an explicit "Convert from BGR to RGB" shouldn't be necessary, Instead, I think that you can change line 276 from this:

use_pixelformat = WUFFS_BASE__PIXEL_FORMAT__BGR

to this:

use_pixelformat = WUFFS_BASE__PIXEL_FORMAT__RGB

Yeah that's my code.
If I use WUFFS_BASE__PIXEL_FORMAT__RGB on line 276 then I get
"Error: base: unsupported pixel swizzler option" on PngSuite-2013jan13/tbbn3p08.png

In addition, despite being relatively unoptimised code, my BGRA->RGBA swizzling seems faster than Wuffs:

wuffs 0.3, WUFFS_BASE__PIXEL_FORMAT__RGB:
-------------------------------------
doDecodeFromBufferWithWuffs took 9.1046 ms
doDecodeFromBufferWithWuffs took 8.9896 ms
doDecodeFromBufferWithWuffs took 9.0422 ms


wuffs 0.3, WUFFS_BASE__PIXEL_FORMAT__BGR and BGRA->RGBA swizzling myself:
-------------------------------------
doDecodeFromBufferWithWuffs took 8.9411 ms
doDecodeFromBufferWithWuffs took 8.8994 ms
doDecodeFromBufferWithWuffs took 8.9056 ms
doDecodeFromBufferWithWuffs took 8.9171 ms

[Ono-Sendai]

I'd also like to know whether any or these #ifdefs trigger for your VS compiler flags:

#if defined(_M_IX86)
#if defined(_M_X64)
#if defined(__AVX__)

Just _M_X64. I'm not building with AVX.

[Ono-Sendai]

wuffs 0.4, alpha 1
------------------
doDecodeFromBufferWithWuffs took 8.8656 ms
doDecodeFromBufferWithWuffs took 8.7829 ms
doDecodeFromBufferWithWuffs took 8.8434 ms

wuffs 0.4, alpha 2
------------------
doDecodeFromBufferWithWuffs took 8.8657 ms
doDecodeFromBufferWithWuffs took 8.8763 ms
doDecodeFromBufferWithWuffs took 8.7356 ms

wuffs 0.4, alpha 3
------------------
doDecodeFromBufferWithWuffs took 8.6924 ms
doDecodeFromBufferWithWuffs took 8.6936 ms
doDecodeFromBufferWithWuffs took 8.7011 ms

wuffs 0.4, alpha 4
------------------
doDecodeFromBufferWithWuffs took 12.211 ms
doDecodeFromBufferWithWuffs took 12.057 ms
doDecodeFromBufferWithWuffs took 12.018 ms

[nigeltao]

I suspect that the slow-down is due to SIMD code no longer being used.

Comparing v0.4.0-alpha.3 and v0.4.0-alpha.4, do any of these #ifdefs trigger?

#if defined (WUFFS_BASE__CPU_ARCH__X86_FAMILY)
#if defined (WUFFS_BASE__CPU_ARCH__X86_64)

---

Also, there's a wuffs_base__cpu_arch__have_x86_sse42 function. What does it return for you (again, comparing alpha.3 and alpha.4):

static inline bool  //
wuffs_base__cpu_arch__have_x86_sse42(void) {
  etc;
}

---

I'm not building with AVX.

Out of curiosity (I'm not familiar with MSVC / Visual Studio), both wuffs-v0.3.c and wuffs-v0.4.c have this line:

#pragma message("Wuffs with MSVC+IX86/X64 needs /arch:AVX for best performance")

Did you notice that at all, when building Wuffs?

[nigeltao]

I suspect that the slow-down is due to SIMD code no longer being used.

That might not be true, though. Fortunately, there's not that many commits between v0.4.0-alpha.3 and v0.4.0-alpha.4. Are you able to switch over from wuffs-v0.4.c to wuffs-unsupported-snapshot.c and bisect these commits:

f1698226 (tag: v0.4.0-alpha.4) release: wuffs gen -version=0.4.0-alpha.4
a27a7598 base: disable SIMD on 32-bit (not 64-bit) x86
4209264a cgen: disable SIMD on 32-bit (not 64-bit) x86
fdd08200 test/3pdata: add note re XZ backdoor test files
d8c3deb0 aux: remove DynIOBuffer::allocated
00a01abc base: avoid some (ptr + len) when ptr is NULL
d7f6b81d base: have empty_foo() functions use NULL ptr
2142aa65 cgen: fix iterate's "ptr + len" when ptr is NULL
8ed5d132 example/gifplayer: tweak try_allocate workbuf len
a9a6d8ac base: make malloc_slice_u8(etc, 0) return empty
f32bfe95 std/crc64: optimize for x86+SSE4.2
5c693c9d (tag: v0.4.0-alpha.3) release: wuffs gen -version=0.4.0-alpha.3

[Ono-Sendai]

I suspect that the slow-down is due to SIMD code no longer being used.

Comparing v0.4.0-alpha.3 and v0.4.0-alpha.4, do any of these #ifdefs trigger?

#if defined (WUFFS_BASE__CPU_ARCH__X86_FAMILY)
#if defined (WUFFS_BASE__CPU_ARCH__X86_64)

WUFFS_BASE__CPU_ARCH__X86_64 is not defined for me.

Also, there's a `wuffs_base__cpu_arch__have_x86_sse42` function. What does it return for you (again, comparing alpha.3 and alpha.4):

static inline bool //
wuffs_base__cpu_arch__have_x86_sse42(void) {
etc;
}

> I'm not building with AVX.

Out of curiosity (I'm not familiar with MSVC / Visual Studio), both `wuffs-v0.3.c` and `wuffs-v0.4.c` have this line:

#pragma message("Wuffs with MSVC+IX86/X64 needs /arch:AVX for best performance")

Did you notice that at all, when building Wuffs?

Yes, and it's kind of annoying :)

Looking at the code, the issue seems to be

#if defined(__AVX__) || defined(__clang__)

preventing SSE from being used.

As mentioned before I'm not building with AVX.

[nigeltao]

WUFFS_BASE__CPU_ARCH__X86_64 is not defined for me.

To be clear, are you saying "WUFFS_BASE__CPU_ARCH__X86_64 is not defined for me" for just v0.4.0-alpha.4 or for both v0.4.0-alpha.3 and v0.4.0-alpha.4?

Again, what does wuffs_base__cpu_arch__have_x86_sse42 return, for both alpha.3 and alpha.4?

---

#pragma message("Wuffs with MSVC+IX86/X64 needs /arch:AVX for best performance")

Did you notice that at all, when building Wuffs?

Yes, and it's kind of annoying :)

Yeah, it's not ideal, but I don't know how to make it better.

Wuffs ships as a "single file C library". And in gcc or clang, code can opt-in to "compile me with SIMD enabled" via an __attribute__ on a function by function basis, by the library writer (me). But if I understand correctly, for Visual Studio, enabling SIMD is on a file by file basis (e.g. the /arch:AVX compile-time switch), by the library user (you). (Yeah, technically, by "file" I mean "translation unit".)

So, for VS, I'd like the single file C library to work out of the box (even if, by default, it's leaving significant performance on the table), and it does, but the wuffs-v0.4.c file itself can't self-configure VS to SIMD-enable the SIMD-using functions, and the #pragma message was the least worst mechanism I could think of to try to raise awareness for VS-using programmers.

---

Sort of tangential to the original post, but as you're concerned about PNG decode performance: if your CPUs are less than 10 years old, then I'm curious how that "8.4 milliseconds" number changes if you do pass /arch:AVX, on either Wuffs alpha.3 and alpha.4.

[Ono-Sendai]

So, for VS, I'd like the single file C library to work out of the box (even if, by default, it's leaving significant performance on the table), and it does, but the wuffs-v0.4.c file itself can't self-configure VS to SIMD-enable the SIMD-using functions, and the #pragma message was the least worst mechanism I could think of to try to raise awareness for VS-using programmers.

Maybe you could add a preprocessor option to suppress the warning? something like
WUFFS_NO_MISSING_AVX_WARNING

[Ono-Sendai]

Sort of tangential to the original post, but as you're concerned about PNG decode performance: if your CPUs are less than 10 years old, then I'm curious how that "8.4 milliseconds" number changes if you do pass /arch:AVX, on either Wuffs alpha.3 and alpha.4.

AVX not set
---------------------------------------------
doDecodeFromBufferWithWuffs took 8.8381 ms
doDecodeFromBufferWithWuffs took 8.7475 ms
doDecodeFromBufferWithWuffs took 8.6410 ms
doDecodeFromBufferWithWuffs took 8.6766 ms


With Advanced Vector Extensions 2 (X86/X64) (/arch:AVX2)
--------------------------------------------------------
doDecodeFromBufferWithWuffs took 8.6368 ms
doDecodeFromBufferWithWuffs took 8.6676 ms
doDecodeFromBufferWithWuffs took 8.5720 ms
doDecodeFromBufferWithWuffs took 8.6433 ms

[Ono-Sendai]

WUFFS_BASE__CPU_ARCH__X86_64 is not defined for me.

To be clear, are you saying "WUFFS_BASE__CPU_ARCH__X86_64 is not defined for me" for just v0.4.0-alpha.4 or for both v0.4.0-alpha.3 and v0.4.0-alpha.4?

It's not defined for me using v0.4.0-alpha.4. Not sure about alpha 3.

[Ono-Sendai]

Again, what does wuffs_base__cpu_arch__have_x86_sse42 return, for both alpha.3 and alpha.4?

I think I have done enough remote debugging for now sorry. I think you need a windows build machine :)

[nigeltao]

I think I have done enough remote debugging for now sorry. I think you need a windows build machine :)

OK, but in that case, I don't expect this bug to be fixed any time soon. Sorry.