Improve decode performance

[basiliscos]

Hi,

I've noted terrible performance in debug mode. So, I started to dig what's the problem... and I found algorithmic no issues in your code. So, the performance degradation is probably related to templated code.

Any way, I did minor changed, which led to some performance boost.

Here is a benchmarks (in Release mode):

before:

------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
BM_protopuf_encode            17.2 ns         17.2 ns     40928792
BM_protopuf_safe_encode       29.7 ns         29.7 ns     22518719
BM_protobuf_encode            68.0 ns         68.0 ns     10340438
BM_protopuf_decode             188 ns          188 ns      3728154
BM_protopuf_safe_decode        217 ns          217 ns      3232510
BM_protobuf_decode             244 ns          244 ns      2906093

After:

------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
BM_protopuf_encode            17.3 ns         17.3 ns     40937909
BM_protopuf_safe_encode       30.5 ns         30.5 ns     22250119
BM_protobuf_encode            69.3 ns         69.3 ns     10183935
BM_protopuf_decode             145 ns          145 ns      4840484
BM_protopuf_safe_decode        162 ns          162 ns      4311272
BM_protobuf_decode             243 ns          243 ns      2895603

[basiliscos]

tests fail due to begin_diff removal. Should I remove it from tests too?

[PragmaTwice]

Hi, thank you for your contribution.

However IMHO, we cannot prefer debug-mode performance than maintainability. Debug-mode performance can be intuitively bad since it may involves some debug-only runtime checks and NO compiler optimizations.

In CMake, Debug mode usually means -O0 -g. If you want debug information but also want some performance, you can use RelWithDebInfo mode instead of Release which usually means -O2 -g and enables lots of compiler optimizations (also you can try -O1 -g if you want more precise debug information).

[PragmaTwice]

need_checks seems useless. check_bytes_span will be an empty function in unsafe mode.

And it should be quite easy for compilers to eliminate.

[basiliscos]

And it should be quite easy for compilers to eliminate.

It can eliminate or can not. It depends on compiler, version, mode etc.

The code above performs that removal independently from compiler, especially it removes the empty call in debug mode. There can be a lot of them.

[PragmaTwice]

Hmm I think another way is to put [[gnu::always_inline]]/[[msvc::forceinline]] into its declaration (check_bytes_span), so that it can always be inlined and eliminated, even in debug mode.

Since I'm a compiler engineer I can guarantee that every C++ compiler can do this (MSVC/Clang/GCC), because it's quite trivial.

[PragmaTwice]

Hmm in every iteration, we get a new b, and the bytes it forwards here may NOT be 1, so --len is not correct IIRC.

[basiliscos]

Oh, yeah, indeed. It seems number of bytes not number of items :-/

[basiliscos]

In CMake, Debug mode usually means -O0 -g. If you want debug information but also want some performance, you can use RelWithDebInfo mode instead of Release which usually means -O2 -g and enables lots of compiler optimizations (also you can try -O1 -g if you want more precise debug information).

That's true, however I want my code to be compiled in full debug mode without optimizations, and as the library is header-only it cannot be easily achieved.