Skip to content

Improve decode performance #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
basiliscos wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve decode performance #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions include/protopuf/array.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ namespace pp {
con.reserve(len);
}

const auto origin_b = b;
decode_value<typename C::value_type> decode_v;
while(begin_diff(b, origin_b) < len) {
while(len) {
if (Mode::get_value_from_result(C::template decode<Mode>(b), decode_v)) {
std::tie(*std::inserter(con, con.end()), b) = std::move(decode_v);
--len;
Comment on lines +85 to +88
Copy link
Owner

@PragmaTwice PragmaTwice Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@basiliscos basiliscos Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else {
return {};
}
Expand Down Expand Up @@ -122,8 +122,10 @@ namespace pp {
uint<8> n = 0;
std::tie(n, b) = decode_len;

if (!Mode::check_bytes_span(b, n)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_bytes_span(b, n)) {
return {};
}
}
Comment on lines +125 to 129
Copy link
Owner

@PragmaTwice PragmaTwice Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@basiliscos basiliscos Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

@PragmaTwice PragmaTwice Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


return Mode::template make_result<decode_skip_result<Mode>>(b.subspan(n));
Expand Down
5 changes: 0 additions & 5 deletions include/protopuf/byte.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ namespace pp {
/// A byte (contiguous) sequence reference (no ownership).
using bytes = std::span<std::byte>;

/// Returns the byte-distance between `begin(a)` and `begin(b)`.
inline constexpr std::size_t begin_diff(bytes a, bytes b) {
// `std::to_address` is used here for MSVC, ref to https://github.com/microsoft/STL/issues/1435
return static_cast<std::size_t>(std::to_address(a.begin()) - std::to_address(b.begin()));
}
}

#endif //PROTOPUF_BYTE_H
6 changes: 5 additions & 1 deletion include/protopuf/coder_mode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ namespace pp {
template<typename T>
using result_type = std::remove_reference_t<T>;

static constexpr bool need_checks = false;

template<typename R, typename... Args>
static constexpr R make_result(Args&&... args) {
return R{std::forward<Args>(args)...};
Expand All @@ -98,6 +100,8 @@ namespace pp {
template<typename T>
using result_type = std::optional<std::remove_reference_t<T>>;

static constexpr bool need_checks = true;

template<typename R, typename... Args>
static constexpr R make_result(Args&&... args) {
return R{std::in_place, std::forward<Args>(args)...};
Expand All @@ -124,4 +128,4 @@ namespace pp {

}

#endif //PROTOPUF_CODER_MODE_H
#endif //PROTOPUF_CODER_MODE_H
12 changes: 8 additions & 4 deletions include/protopuf/int.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,10 @@ namespace pp {

template <coder_mode Mode = safe_mode>
static constexpr encode_result<Mode> encode(T i, bytes b) {
if (!Mode::check_bytes_span(b, N)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_bytes_span(b, N)) {
return {};
}
}

int_to_bytes<N>(i, b.subspan<0, N>());
Expand All @@ -209,8 +211,10 @@ namespace pp {

template <coder_mode Mode = safe_mode>
static constexpr decode_result<T, Mode> decode(bytes b) {
if (!Mode::check_bytes_span(b, N)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_bytes_span(b, N)) {
return {};
}
}

return Mode::template make_result<decode_result<T, Mode>>(bytes_to_int<N>(b.subspan<0, N>()), b.subspan<N>());
Expand Down
10 changes: 6 additions & 4 deletions include/protopuf/message.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,13 +489,13 @@ namespace pp {
std::size_t len = 0;
std::tie(len, b) = decod_len;

const auto origin_b = b;
while(begin_diff(b, origin_b) < len) {
while(len) {
std::pair<bytes, bool> bytes_with_next;
if (!Mode::get_value_from_result(decode_map<Mode, T>.decode(v, b), bytes_with_next)) {
return {};
}

--len;
bool next = true;
std::tie(b, next) = bytes_with_next;

Expand Down Expand Up @@ -528,8 +528,10 @@ namespace pp {
uint<8> n = 0;
std::tie(n, b) = decode_len;

if (!Mode::check_bytes_span(b, n)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_bytes_span(b, n)) {
return {};
}
}

return Mode::template make_result<decode_skip_result<Mode>>(b.subspan(n));
Expand Down
24 changes: 16 additions & 8 deletions include/protopuf/skip.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@ namespace pp {

template <coder_mode Mode = safe_mode>
static constexpr decode_skip_result<Mode> decode_skip(bytes b) {
if (!Mode::check_bytes_span(b, sizeof(T))) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_bytes_span(b, sizeof(T))) {
return {};
}
}
return Mode::template make_result<decode_skip_result<Mode>>(b.subspan<sizeof(T)>());
}
Expand All @@ -100,8 +102,10 @@ namespace pp {

template <coder_mode Mode = safe_mode>
static constexpr decode_skip_result<Mode> decode_skip(bytes b) {
if (!Mode::check_bytes_span(b, sizeof(T))) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_bytes_span(b, sizeof(T))) {
return {};
}
}
return Mode::template make_result<decode_skip_result<Mode>>(b.subspan<sizeof(T)>());
}
Expand All @@ -127,13 +131,17 @@ namespace pp {
auto iter = b.begin();
const auto end = b.end();

if (!Mode::check_iterator(iter, end)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_iterator(iter, end)) {
return {};
}
}

while((*iter++ >> 7) == 1_b) {
if (!Mode::check_iterator(iter, end)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_iterator(iter, end)) {
return {};
}
}
}

Expand Down
18 changes: 12 additions & 6 deletions include/protopuf/varint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ namespace pp {
const auto end = s.end();

do {
if (!Mode::check_iterator(iter, end)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_iterator(iter, end)) {
return {};
}
}

*iter = 0b1000'0000_b | std::byte(n);
Expand All @@ -69,17 +71,21 @@ namespace pp {
auto iter = s.begin();
const auto end = s.end();

if (!Mode::check_iterator(iter, end)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_iterator(iter, end)) {
return {};
}
}

std::size_t i = 0;
while((*iter >> 7) == 1_b) {
n |= static_cast<T>(static_cast<T>(*iter & 0b0111'1111_b) << 7*i);
++iter, ++i;

if (!Mode::check_iterator(iter, end)) {
return {};
if constexpr (Mode::need_checks) {
if (!Mode::check_iterator(iter, end)) {
return {};
}
}
}
n |= static_cast<T>(static_cast<T>(*iter++) << 7 * i);
Expand Down
Loading