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

async_msg: take ownership of filename/funcname #2935

Open
wants to merge 1 commit into
base: v1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ jobs:
- { compiler: gcc, version: 12, build_type: Release, cppstd: 20 }
- { compiler: clang, version: 12, build_type: Debug, cppstd: 17, asan: OFF }
- { compiler: clang, version: 15, build_type: Release, cppstd: 20, asan: OFF }
async_owning_sourceloc_strings:
- ON
- OFF
container:
image: ${{ matrix.config.compiler == 'clang' && 'teeks99/clang-ubuntu' || matrix.config.compiler }}:${{ matrix.config.version }}
name: "${{ matrix.config.compiler}} ${{ matrix.config.version }} (C++${{ matrix.config.cppstd }}, ${{ matrix.config.build_type }})"
Expand Down Expand Up @@ -51,6 +54,7 @@ jobs:
-DSPDLOG_BUILD_BENCH=OFF \
-DSPDLOG_BUILD_TESTS=ON \
-DSPDLOG_BUILD_TESTS_HO=OFF \
-DSPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS=${{ matrix.async_owning_sourceloc_strings }} \
-DSPDLOG_SANITIZE_ADDRESS=${{ matrix.config.asan || 'ON' }}
make -j2
ctest -j2 --output-on-failure
Expand Down
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ option(SPDLOG_SANITIZE_ADDRESS "Enable address sanitizer in tests" OFF)
# warning options
option(SPDLOG_BUILD_WARNINGS "Enable compiler warnings" OFF)

# owning sourceloc strings options
option(SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS "Async loggers take owership of source_loc strings" OFF)


# install options
option(SPDLOG_SYSTEM_INCLUDES "Include as system headers (skip for clang-tidy)." OFF)
option(SPDLOG_INSTALL "Generate the install target" ${SPDLOG_MASTER_PROJECT})
Expand Down Expand Up @@ -248,6 +252,7 @@ foreach(
SPDLOG_NO_TLS
SPDLOG_NO_ATOMIC_LEVELS
SPDLOG_DISABLE_DEFAULT_LOGGER
SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
SPDLOG_USE_STD_FORMAT)
if(${SPDLOG_OPTION})
target_compile_definitions(spdlog PUBLIC ${SPDLOG_OPTION})
Expand Down
42 changes: 34 additions & 8 deletions include/spdlog/details/thread_pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <chrono>
#include <functional>
#include <memory>
#include <string>
#include <thread>
#include <vector>

Expand All @@ -34,29 +35,48 @@ struct async_msg : log_msg_buffer {
// should only be moved in or out of the queue..
async_msg(const async_msg &) = delete;

// support for vs2013 move
#if defined(_MSC_VER) && _MSC_VER <= 1800
async_msg(async_msg &&other)
: log_msg_buffer(std::move(other)),
msg_type(other.msg_type),
worker_ptr(std::move(other.worker_ptr)) {}
worker_ptr(std::move(other.worker_ptr)) {
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
source.filename = filename_string.c_str();
source.funcname = function_string.c_str();
#endif
}

async_msg &operator=(async_msg &&other) {
*static_cast<log_msg_buffer *>(this) = std::move(other);
msg_type = other.msg_type;
worker_ptr = std::move(other.worker_ptr);
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
source.filename = filename_string.c_str();
source.funcname = function_string.c_str();
#endif
return *this;
}
#else // (_MSC_VER) && _MSC_VER <= 1800
async_msg(async_msg &&) = default;
async_msg &operator=(async_msg &&) = default;
#endif

// construct from log_msg with given type
async_msg(async_logger_ptr &&worker, async_msg_type the_type, const details::log_msg &m)
: log_msg_buffer{m},
msg_type{the_type},
worker_ptr{std::move(worker)} {}
worker_ptr{std::move(worker)} {
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
// take ownership of filename/funcname
if (m.source.filename) {
filename_string = std::string{m.source.filename};
source.filename = filename_string.c_str();
} else {
source.filename = nullptr;
}
if (m.source.funcname) {
function_string = std::string{m.source.funcname};
source.funcname = function_string.c_str();
} else {
source.funcname = nullptr;
}
#endif
}

async_msg(async_logger_ptr &&worker, async_msg_type the_type)
: log_msg_buffer{},
Expand All @@ -65,6 +85,12 @@ struct async_msg : log_msg_buffer {

explicit async_msg(async_msg_type the_type)
: async_msg{nullptr, the_type} {}

#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
// source.filename/funcname always need to point to the member strings
std::string filename_string;
std::string function_string;
#endif
};

class SPDLOG_API thread_pool {
Expand Down