Skip to content
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.

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

minor DeferredFormatCodec, DirectFormatCodec improvements #676

Merged
merged 2 commits into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 48 additions & 7 deletions include/quill/DeferredFormatCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
#include "quill/core/DynamicFormatArgStore.h"
#include "quill/core/InlinedVector.h"

#include <cstddef>
#include <cstdint>
#include <cstring>
#include <new>
#include <type_traits>
#include <utility>

QUILL_BEGIN_NAMESPACE

Expand Down Expand Up @@ -81,8 +85,6 @@ QUILL_BEGIN_NAMESPACE
template <typename T>
struct DeferredFormatCodec
{
// Checks like this Will fail for types with private ctors and friends declarations
// static_assert(std::is_copy_constructible_v<T>, "T must be copy constructible");
static constexpr bool is_trivially_copyable = std::is_trivially_copyable_v<T>;

static size_t compute_encoded_size(detail::SizeCacheVector&, T const&) noexcept
Expand All @@ -93,11 +95,12 @@ struct DeferredFormatCodec
}
else
{
return sizeof(T) + alignof(T);
// If it’s misaligned, the worst-case scenario is when the pointer is off by one byte from an alignment boundary
return sizeof(T) + alignof(T) - 1;
}
}

static void encode(std::byte*& buffer, detail::SizeCacheVector const&, uint32_t&, T const& arg) noexcept
static void encode(std::byte*& buffer, detail::SizeCacheVector const&, uint32_t&, T const& arg)
{
if constexpr (is_trivially_copyable)
{
Expand All @@ -108,14 +111,15 @@ struct DeferredFormatCodec
{
auto aligned_ptr = align_pointer(buffer, alignof(T));
new (static_cast<void*>(aligned_ptr)) T(arg);
buffer += sizeof(T) + alignof(T);
buffer += sizeof(T) + alignof(T) - 1;
}
}

static T decode_arg(std::byte*& buffer)
{
if constexpr (is_trivially_copyable)
{
static_assert(is_default_constructible<T>::value, "T is not default-constructible!");
T arg;

// Cast to void* to silence compiler warning about private members
Expand All @@ -127,9 +131,10 @@ struct DeferredFormatCodec
else
{
auto aligned_ptr = align_pointer(buffer, alignof(T));
auto* tmp = reinterpret_cast<T*>(aligned_ptr);
auto* tmp = std::launder(reinterpret_cast<T*>(aligned_ptr));

// Take a copy
static_assert(is_copy_constructible<T>::value, "T is not copy-constructible!");
T arg{*tmp};

if constexpr (!std::is_trivially_destructible_v<T>)
Expand All @@ -138,17 +143,53 @@ struct DeferredFormatCodec
tmp->~T();
}

buffer += sizeof(T) + alignof(T);
buffer += sizeof(T) + alignof(T) - 1;
return arg;
}
}

static void decode_and_store_arg(std::byte*& buffer, DynamicFormatArgStore* args_store)
{
static_assert(is_move_constructible<T>::value, "T is not move-constructible!");
args_store->push_back(decode_arg(buffer));
}

private:
// These trait implementations will take the friend declaration into account

// Default constructible check
template <typename U, typename = void>
struct is_default_constructible : std::false_type
{
};

template <typename U>
struct is_default_constructible<U, std::void_t<decltype(U())>> : std::true_type
{
};

// Copy constructible check: tests if we can call U(const U&)
template <typename U, typename = void>
struct is_copy_constructible : std::false_type
{
};

template <typename U>
struct is_copy_constructible<U, std::void_t<decltype(U(std::declval<U const&>()))>> : std::true_type
{
};

// Move constructible check: tests if we can call U(U&&)
template <typename U, typename = void>
struct is_move_constructible : std::false_type
{
};

template <typename U>
struct is_move_constructible<U, std::void_t<decltype(U(std::declval<U&&>()))>> : std::true_type
{
};

static std::byte* align_pointer(void* pointer, size_t alignment) noexcept
{
return reinterpret_cast<std::byte*>((reinterpret_cast<uintptr_t>(pointer) + (alignment - 1ul)) &
Expand Down
22 changes: 13 additions & 9 deletions include/quill/DirectFormatCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
#include "quill/core/DynamicFormatArgStore.h"
#include "quill/core/InlinedVector.h"

#include <cstddef>
#include <cstdint>
#include <cstring>
#include <string_view>

QUILL_BEGIN_NAMESPACE

Expand Down Expand Up @@ -83,20 +86,21 @@ QUILL_BEGIN_NAMESPACE
template <typename T>
struct DirectFormatCodec
{
static size_t compute_encoded_size(quill::detail::SizeCacheVector& conditional_arg_size_cache, T const& arg) noexcept
static size_t compute_encoded_size(quill::detail::SizeCacheVector& conditional_arg_size_cache, T const& arg)
{
return sizeof(uint32_t) +
conditional_arg_size_cache.push_back(static_cast<uint32_t>(fmtquill::formatted_size("{}", arg)));
// The computed size includes the size of the length field
return sizeof(uint32_t) +
conditional_arg_size_cache.push_back(static_cast<uint32_t>(fmtquill::formatted_size("{}", arg)));
}

static void encode(std::byte*& buffer, quill::detail::SizeCacheVector const& conditional_arg_size_cache,
uint32_t& conditional_arg_size_cache_index, T const& arg) noexcept
uint32_t& conditional_arg_size_cache_index, T const& arg)
{
uint32_t const len = conditional_arg_size_cache[conditional_arg_size_cache_index++];
std::memcpy(buffer, &len, sizeof(len));
buffer += sizeof(len);
fmtquill::format_to_n(reinterpret_cast<char*>(buffer), len, "{}", arg);
buffer += len;
uint32_t const len = conditional_arg_size_cache[conditional_arg_size_cache_index++];
std::memcpy(buffer, &len, sizeof(len));
buffer += sizeof(len);
fmtquill::format_to_n(reinterpret_cast<char*>(buffer), len, "{}", arg);
buffer += len;
}

static std::string_view decode_arg(std::byte*& buffer)
Expand Down
Loading