Skip to content

Commit

Permalink
minor DeferredFormatCodec, DirectFormatCodec improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
odygrd authored Feb 16, 2025
1 parent 6e81cca commit 183ec22
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 16 deletions.
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

0 comments on commit 183ec22

Please sign in to comment.