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

do not include unused x86intrin headers, fix msvc compile error #300

Merged
merged 5 commits into from
Jun 1, 2023
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
7 changes: 7 additions & 0 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- [v2.9.2](#v292)
- [v2.9.1](#v291)
- [v2.9.0](#v290)
- [v2.8.0](#v280)
Expand Down Expand Up @@ -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++
Expand Down
6 changes: 5 additions & 1 deletion quill/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,11 @@ target_compile_options(${TARGET_NAME} PRIVATE
$<$<CXX_COMPILER_ID:MSVC>:/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 ()

Expand Down
2 changes: 1 addition & 1 deletion quill/include/quill/Quill.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 **/
Expand Down
3 changes: 3 additions & 0 deletions quill/include/quill/detail/misc/Rdtsc.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
// assume x86-64 ..
#if defined(_WIN32)
#include <intrin.h>
#elif (defined(__GNUC__) && __GNUC__ > 10) || (defined(__clang_major__) && __clang_major__ > 11)
#include <x86gprintrin.h>
#else
// older compiler versions do not have <x86gprintrin.h>
#include <x86intrin.h>
#endif
#endif
Expand Down
18 changes: 13 additions & 5 deletions quill/include/quill/detail/spsc_queue/BoundedQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,16 @@
#include <stdexcept>

#if defined(QUILL_X86ARCH)
#include <emmintrin.h>
#include <immintrin.h>
#include <x86intrin.h>
#if defined(_WIN32)
#include <intrin.h>
#elif (defined(__GNUC__) && __GNUC__ > 10) || (defined(__clang_major__) && __clang_major__ > 11)
#include <emmintrin.h>
#include <x86gprintrin.h>
#else
// older compiler versions do not have <x86gprintrin.h>
#include <immintrin.h>
#include <x86intrin.h>
#endif
#endif

namespace quill::detail
Expand Down Expand Up @@ -62,7 +69,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<char const*>(_storage + (CACHE_LINE_SIZE * i)), _MM_HINT_T0);
}
#endif
}
Expand Down Expand Up @@ -99,7 +106,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<char const*>(_storage + ((_writer_pos + (CACHE_LINE_SIZE * 10)) & _mask)),
_MM_HINT_T0);
#endif

// set the atomic flag so the reader can see write
Expand Down
54 changes: 29 additions & 25 deletions quill/test/BoundedQueueTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -53,6 +56,32 @@ TEST_CASE("read_write_buffer")
REQUIRE_FALSE(res);
}

TEST_CASE("bounded_queue_integer_overflow")
{
BoundedQueueImpl<uint8_t> buffer{128};
size_t const iterations = static_cast<size_t>(std::numeric_limits<uint8_t>::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<uint8_t>(to_write.length()) + 1);
std::strncpy(reinterpret_cast<char*>(r), to_write.data(), to_write.length() + 1);
buffer.finish_write(static_cast<uint8_t>(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<uint8_t>(to_write.length()) + 1);
REQUIRE_STREQ(result, to_write.data());
buffer.finish_read(static_cast<uint8_t>(to_write.length()) + 1);
buffer.commit_read();
}
}
#endif

TEST_CASE("read_write_multithreaded_plain_ints")
{
BoundedQueue buffer{131'072};
Expand Down Expand Up @@ -105,29 +134,4 @@ TEST_CASE("read_write_multithreaded_plain_ints")
consumer_thread.join();
}

TEST_CASE("bounded_queue_integer_overflow")
{
BoundedQueueImpl<uint8_t> buffer{128};
size_t const iterations = static_cast<size_t>(std::numeric_limits<uint8_t>::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<uint8_t>(to_write.length()) + 1);
std::strncpy(reinterpret_cast<char*>(r), to_write.data(), to_write.length() + 1);
buffer.finish_write(static_cast<uint8_t>(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<uint8_t>(to_write.length()) + 1);
REQUIRE_STREQ(result, to_write.data());
buffer.finish_read(static_cast<uint8_t>(to_write.length()) + 1);
buffer.commit_read();
}
}

TEST_SUITE_END();
2 changes: 1 addition & 1 deletion quill/test/UnboundedQueueTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]()
Expand Down