From 11e7d04da4923ec4a8cd82a3fa594c5a57d729d2 Mon Sep 17 00:00:00 2001 From: Odysseas Georgoudis Date: Wed, 31 May 2023 23:12:55 +0100 Subject: [PATCH 1/5] do not include unused x86intrin headers, fix msvc compile error --- CHANGELOG.md | 12 ++++++++++-- quill/CMakeLists.txt | 6 +++++- quill/include/quill/Quill.h | 2 +- quill/include/quill/detail/misc/Rdtsc.h | 2 +- quill/include/quill/detail/spsc_queue/BoundedQueue.h | 7 +++---- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0031fb9..b2eceb73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ +- [v2.9.2](#v292) - [v2.9.1](#v291) - [v2.9.0](#v290) - [v2.8.0](#v280) @@ -42,14 +43,21 @@ - [v1.1.0](#v110) - [v1.0.0](#v100) +## v2.9.2 + +- Fix increased compile times due to `x86intrin` headers. ([#298](https://github.com/odygrd/quill/pull/298)) +- Fix compile error when using `QUILL_X86ARCH` on windows. + ## v2.9.1 - Removed `CMAKE_INSTALL_RPATH` from cmake. ([#284](https://github.com/odygrd/quill/pull/284)) - Fix compile warning on Apple M1. ([#291](https://github.com/odygrd/quill/pull/291)) - Update bundled `libfmt` to `v10.0.0` - Fix for `CMAKE_MODULE_PATH` ([#295](https://github.com/odygrd/quill/pull/295)) -- Fixed a bug in `TimeRotatingFileHandler` when `quill::FilenameAppend::None` is used. ([#296](https://github.com/odygrd/quill/pull/296)) -- Fixed `TimeRotatingFileHandler` and `RotatingFileHandler` to work when `/dev/null` is used as a filename ([#297](https://github.com/odygrd/quill/pull/297)) +- Fixed a bug in `TimeRotatingFileHandler` when `quill::FilenameAppend::None` is + used. ([#296](https://github.com/odygrd/quill/pull/296)) +- Fixed `TimeRotatingFileHandler` and `RotatingFileHandler` to work when `/dev/null` is used as a + filename ([#297](https://github.com/odygrd/quill/pull/297)) - Added `NullHandler` that can be used to discard the logs. For example: ```c++ diff --git a/quill/CMakeLists.txt b/quill/CMakeLists.txt index c6316d97..628c4e3d 100644 --- a/quill/CMakeLists.txt +++ b/quill/CMakeLists.txt @@ -183,7 +183,11 @@ target_compile_options(${TARGET_NAME} PRIVATE $<$:/W4 /wd4324 /wd4189 /wd4996 /wd4100 /wd4127>) if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") - target_compile_options(${TARGET_NAME} PRIVATE /WX /EHsc) + if (NOT QUILL_NO_EXCEPTIONS) + target_compile_options(${TARGET_NAME} PUBLIC /EHsc) + endif () + + target_compile_options(${TARGET_NAME} PRIVATE /WX) target_compile_definitions(${TARGET_NAME} PRIVATE _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING) endif () diff --git a/quill/include/quill/Quill.h b/quill/include/quill/Quill.h index bbdd8ebd..b8229128 100644 --- a/quill/include/quill/Quill.h +++ b/quill/include/quill/Quill.h @@ -31,7 +31,7 @@ namespace quill /** Version Info **/ constexpr uint32_t VersionMajor{2}; constexpr uint32_t VersionMinor{9}; -constexpr uint32_t VersionPatch{1}; +constexpr uint32_t VersionPatch{2}; constexpr uint32_t Version{VersionMajor * 10000 + VersionMinor * 100 + VersionPatch}; /** forward declarations **/ diff --git a/quill/include/quill/detail/misc/Rdtsc.h b/quill/include/quill/detail/misc/Rdtsc.h index d4396b8a..ad73cfe7 100644 --- a/quill/include/quill/detail/misc/Rdtsc.h +++ b/quill/include/quill/detail/misc/Rdtsc.h @@ -16,7 +16,7 @@ #if defined(_WIN32) #include #else - #include + #include #endif #endif diff --git a/quill/include/quill/detail/spsc_queue/BoundedQueue.h b/quill/include/quill/detail/spsc_queue/BoundedQueue.h index 67d2af71..449e7969 100644 --- a/quill/include/quill/detail/spsc_queue/BoundedQueue.h +++ b/quill/include/quill/detail/spsc_queue/BoundedQueue.h @@ -15,8 +15,6 @@ #if defined(QUILL_X86ARCH) #include - #include - #include #endif namespace quill::detail @@ -62,7 +60,7 @@ class BoundedQueueImpl for (uint64_t i = 0; i < cache_lines; ++i) { - _mm_prefetch(_storage + (CACHE_LINE_SIZE * i), _MM_HINT_T0); + _mm_prefetch(reinterpret_cast(_storage + (CACHE_LINE_SIZE * i)), _MM_HINT_T0); } #endif } @@ -99,7 +97,8 @@ class BoundedQueueImpl _flush_cachelines(_last_flushed_writer_pos, _writer_pos); // prefetch a future cache line - _mm_prefetch(_storage + ((_writer_pos + (CACHE_LINE_SIZE * 10)) & _mask), _MM_HINT_T0); + _mm_prefetch(reinterpret_cast(_storage + ((_writer_pos + (CACHE_LINE_SIZE * 10)) & _mask)), + _MM_HINT_T0); #endif // set the atomic flag so the reader can see write From 43232d40cc3b703879158e700630bad0bc8fbfd9 Mon Sep 17 00:00:00 2001 From: Odysseas Georgoudis Date: Wed, 31 May 2023 23:36:04 +0100 Subject: [PATCH 2/5] fix --- quill/include/quill/detail/misc/Rdtsc.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/quill/include/quill/detail/misc/Rdtsc.h b/quill/include/quill/detail/misc/Rdtsc.h index ad73cfe7..942f455f 100644 --- a/quill/include/quill/detail/misc/Rdtsc.h +++ b/quill/include/quill/detail/misc/Rdtsc.h @@ -15,8 +15,11 @@ // assume x86-64 .. #if defined(_WIN32) #include - #else + #elif (defined(__GNUC__) && __GNUC__ > 10) || (defined(__clang_major__) && __clang_major__ > 11) #include + #else + // older compiler versions do not have + #include #endif #endif From 8b36dca97141e350bf7c469afabb323fc26e9c66 Mon Sep 17 00:00:00 2001 From: Odysseas Georgoudis Date: Wed, 31 May 2023 23:54:04 +0100 Subject: [PATCH 3/5] fix --- quill/include/quill/detail/misc/Rdtsc.h | 2 +- quill/include/quill/detail/spsc_queue/BoundedQueue.h | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/quill/include/quill/detail/misc/Rdtsc.h b/quill/include/quill/detail/misc/Rdtsc.h index 942f455f..e4fb8bee 100644 --- a/quill/include/quill/detail/misc/Rdtsc.h +++ b/quill/include/quill/detail/misc/Rdtsc.h @@ -19,7 +19,7 @@ #include #else // older compiler versions do not have - #include + #include #endif #endif diff --git a/quill/include/quill/detail/spsc_queue/BoundedQueue.h b/quill/include/quill/detail/spsc_queue/BoundedQueue.h index 449e7969..d0b5e346 100644 --- a/quill/include/quill/detail/spsc_queue/BoundedQueue.h +++ b/quill/include/quill/detail/spsc_queue/BoundedQueue.h @@ -14,7 +14,16 @@ #include #if defined(QUILL_X86ARCH) - #include + #if defined(_WIN32) + #include + #elif (defined(__GNUC__) && __GNUC__ > 10) || (defined(__clang_major__) && __clang_major__ > 11) + #include + #include + #else + // older compiler versions do not have + #include + #include + #endif #endif namespace quill::detail From ead350770636f5284d07962e72444196a45f34b6 Mon Sep 17 00:00:00 2001 From: Odysseas Georgoudis Date: Thu, 1 Jun 2023 00:04:48 +0100 Subject: [PATCH 4/5] add gcc-12 build --- .github/workflows/linux.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index c0f8e67a..4624251f 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -34,6 +34,13 @@ jobs: with_tests: ON install: sudo apt install g++-8 + - cxx: g++-12 + build_type: Release + std: 23 + os: ubuntu-22.04 + with_tests: ON + cmake_options: -DCMAKE_CXX_FLAGS:STRING="-DQUILL_X86ARCH -march=native" + # Build as shared library - cxx: g++-10 build_type: Release From 5af354d79720143cbc17980c36e3defdc5ef0810 Mon Sep 17 00:00:00 2001 From: Odysseas Georgoudis Date: Thu, 1 Jun 2023 00:29:15 +0100 Subject: [PATCH 5/5] fix tests for QUILL_X86ARCH --- quill/test/BoundedQueueTest.cpp | 54 +++++++++++++++++-------------- quill/test/UnboundedQueueTest.cpp | 2 +- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/quill/test/BoundedQueueTest.cpp b/quill/test/BoundedQueueTest.cpp index 930c3d00..0a2deb8a 100644 --- a/quill/test/BoundedQueueTest.cpp +++ b/quill/test/BoundedQueueTest.cpp @@ -11,6 +11,9 @@ TEST_SUITE_BEGIN("BoundedQueue"); using namespace quill::detail; +#if !defined(QUILL_X86ARCH) +// QUILL_X86ARCH requires at least a queue capacity of 1024 and those tests are using a smaller number + TEST_CASE("read_write_buffer") { BoundedQueue buffer{64u}; @@ -53,6 +56,32 @@ TEST_CASE("read_write_buffer") REQUIRE_FALSE(res); } +TEST_CASE("bounded_queue_integer_overflow") +{ + BoundedQueueImpl buffer{128}; + size_t const iterations = static_cast(std::numeric_limits::max()) * 8ull; + + for (size_t i = 0; i < iterations; ++i) + { + std::string to_write{"test"}; + to_write += std::to_string(i); + std::byte* r = buffer.prepare_write(static_cast(to_write.length()) + 1); + std::strncpy(reinterpret_cast(r), to_write.data(), to_write.length() + 1); + buffer.finish_write(static_cast(to_write.length()) + 1); + buffer.commit_write(); + + // now read + std::byte* w = buffer.prepare_read(); + REQUIRE(w); + char result[256]; + std::memcpy(&result[0], w, static_cast(to_write.length()) + 1); + REQUIRE_STREQ(result, to_write.data()); + buffer.finish_read(static_cast(to_write.length()) + 1); + buffer.commit_read(); + } +} +#endif + TEST_CASE("read_write_multithreaded_plain_ints") { BoundedQueue buffer{131'072}; @@ -105,29 +134,4 @@ TEST_CASE("read_write_multithreaded_plain_ints") consumer_thread.join(); } -TEST_CASE("bounded_queue_integer_overflow") -{ - BoundedQueueImpl buffer{128}; - size_t const iterations = static_cast(std::numeric_limits::max()) * 8ull; - - for (size_t i = 0; i < iterations; ++i) - { - std::string to_write{"test"}; - to_write += std::to_string(i); - std::byte* r = buffer.prepare_write(static_cast(to_write.length()) + 1); - std::strncpy(reinterpret_cast(r), to_write.data(), to_write.length() + 1); - buffer.finish_write(static_cast(to_write.length()) + 1); - buffer.commit_write(); - - // now read - std::byte* w = buffer.prepare_read(); - REQUIRE(w); - char result[256]; - std::memcpy(&result[0], w, static_cast(to_write.length()) + 1); - REQUIRE_STREQ(result, to_write.data()); - buffer.finish_read(static_cast(to_write.length()) + 1); - buffer.commit_read(); - } -} - TEST_SUITE_END(); diff --git a/quill/test/UnboundedQueueTest.cpp b/quill/test/UnboundedQueueTest.cpp index fd1262dd..2937c49b 100644 --- a/quill/test/UnboundedQueueTest.cpp +++ b/quill/test/UnboundedQueueTest.cpp @@ -12,7 +12,7 @@ using namespace quill::detail; TEST_CASE("read_write_multithreaded_plain_ints") { - UnboundedQueue buffer{2}; + UnboundedQueue buffer{1024}; std::thread producer_thread( [&buffer]()