V2: modernize, performance improments#89
Closed
andibacher wants to merge 60 commits into
Closed
Conversation
- Update C++ standard from C++17 to C++20 - Fix thread-safety: replace incorrect volatile with std::atomic - Modernize memory management: use std::unique_ptr instead of raw pointers - Apply C++20 spaceship operator in Level class (6 operators → 1) - Add [[nodiscard]], constexpr, and noexcept attributes - Use strongly typed enums (enum : uint8_t) - Remove C++17 conditional compilation blocks - Fix naming convention: smart pointers use 'mp' prefix consistently - Update compiler requirements (GCC 10+, Clang 10+, MSVC 19.29+) BREAKING CHANGE: Requires C++20-capable compiler Files changed: 17 core files API: Backward compatible (internal changes only) Modernize codebase to C++20 - Update C++ standard from C++17 to C++20 - Fix thread-safety: replace incorrect volatile with std::atomic - Modernize memory management: use std::unique_ptr instead of raw pointers - Apply C++20 spaceship operator in Level class (6 operators → 1) - Add [[nodiscard]], constexpr, and noexcept attributes - Use strongly typed enums (enum : uint8_t) - Remove C++17 conditional compilation blocks - Update compiler requirements (GCC 10+, Clang 10+, MSVC 19.29+) BREAKING CHANGE: Requires C++20-capable compiler Files changed: 15+ core files API: Backward compatible (internal changes only)
Apply modern C++20 algorithms and eliminate legacy preprocessor conditionals. Changes: - Use std::find_if in appenderattachable.cpp (clearer intent) - Eliminate temp container in dailyfileappender.cpp (memory savings) - Add named predicates in hierarchy.cpp (better testability) - Remove 13 C++17 conditional blocks (#if __cplusplus >= 201703L) - Replace std::as_const with const auto& (simpler, equally safe) Files changed: 9 Lines: +25, -40 (net -15) Impact: - Improved code clarity and maintainability - Reduced memory allocations in file operations - Removed all preprocessor conditionals for Qt container iteration - No breaking changes (internal implementation only) See ALGORITHMS_IMPLEMENTATION_COMPLETE.md for details.
Reserve 256 bytes in PatternFormatter::format() to avoid multiple reallocations during string building. Reduces allocations especially for complex pattern formatting. Benchmark results: - Very complex pattern (10k msgs): 4.0ms -> 3.5ms (+12.5% faster) - Complex pattern (10k msgs): 3.9ms -> 4.0ms (neutral) - Simple pattern (10k msgs): 4.0ms -> 4.1ms (neutral) Modified: src/log4qt/helpers/patternformatter.cpp
Add move constructors and move assignment operators to LoggingEvent for better C++11/14 compliance and potential zero-copy optimization. Use append() instead of operator+= for consistency. Changes: - LoggingEvent: Add defaulted move constructor and move assignment - LoggingEvent: Add explicit copy constructor/assignment (rule of five) - LoggingEvent: Add rvalue reference constructors for zero-copy - LoggingEvent: Use operator% for string concatenation - WriterAppender: Use const reference to avoid QString copy No measurable performance impact in benchmarks, but improves code quality and follows modern C++ best practices. Modified: - src/log4qt/loggingevent.h - src/log4qt/loggingevent.cpp - src/log4qt/writerappender.cpp
Replace QStringLiteral with modern u"..."_s operator for compile-time string literals. Changes: - Add Qt::StringLiterals namespace using directive for Qt 6.4+ - Add LOG4QT_LITERAL macro for version compatibility - Replace 153+ QStringLiteral calls with u"..."_s syntax Performance impact: - Very complex pattern: 3.5ms -> 3.8ms (neutral, within variance) - Overall timing: 6236ms -> 6112ms (2% faster total) - All 25 tests passing Benefits: - Modern C++/Qt 6 best practices - Better code readability - Compile-time string construction Modified: 50+ files in src/log4qt/
Implement TimestampProvider with configurable thread-local caching to reduce system call overhead. Problem: - Every LoggingEvent called QDateTime::currentMSecsSinceEpoch() (~500ns system call) - At 10,000 msgs/sec = 5ms overhead in timestamp calls - No caching mechanism existed Solution: - New TimestampProvider class with thread-local cache - Default 1ms cache window (configurable 0-100ms) - 99% reduction in system calls for high-frequency logging - Thread-safe via thread_local (zero locks) Performance gains: - Simple logging (10k msgs): 4.1ms -> 4.0ms (2.4% faster) - Overall test suite: 6112ms -> 5994ms (1.9% faster) - High-frequency logging: 5% CPU savings at 100k msg/s - All 25 tests passing Benefits: - Configurable precision vs performance trade-off - Zero API changes or breaking changes - Scales perfectly with thread count Modified: - src/log4qt/helpers/timestampprovider.h (new) - src/log4qt/helpers/timestampprovider.cpp (new) - src/log4qt/loggingevent.cpp - src/log4qt/CMakeLists.txt
…mart pointers P1 modernization changes: 1. Replace volatile with std::atomic (correctness fix): - fileappender.h: mAppendFile, mBufferedIo - binaryfileappender.h: mAppendFile, mBufferedIo - logger.h: mAdditivity - writerappender.h: mImmediateFlush - telnetappender.h: mImmediateFlush - consoleappender.h: mTarget - listappender.h: mConfiguratorList, mMaxCount - log4qt.h: update documentation comment 2. Replace raw new/delete with smart pointers (memory safety): - binaryfileappender: mFile, mDataStream -> std::unique_ptr - fileappender: mFile, mTextStream -> std::unique_ptr - consoleappender: mtextStream -> std::unique_ptr - configuratorhelper: mConfigurationFileWatch -> std::unique_ptr - systemlogappender: char* ident -> std::string - colorconsoleappender: new wchar_t[] -> std::vector<wchar_t> - patternformatter: QList<PatternConverter*> -> std::vector<std::unique_ptr<PatternConverter>> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…used]], fold expressions
1. Replace #define color macros with constexpr (colorconsoleappender.cpp):
- 21 NIX_* macros → constexpr int
- 35 WIN_* macros → constexpr WORD (type-safe)
2. Replace typedef with using (modern type aliases):
- factory.h: AppenderFactoryFunc, FilterFactoryFunc, LayoutFactoryFunc
- configuratorhelper.h: ConfigureFunc
3. Add [[nodiscard]] to getter/query methods:
- logger.h: additivity, level, name, effectiveLevel, isXxxEnabled, etc.
- appender.h: filter, name, layout, requiresLayout
- appenderskeleton.h: filter, layout, isActive, isClosed, name, threshold
- layout.h: contentType, footer, header, name
- loggingevent.h: all 17 getter methods
- filter.h: next, decide
- logmanager.h: 14 static getter/query methods
4. Replace Q_UNUSED() with [[maybe_unused]] (9 occurrences, 8 files):
- binarylayout.cpp, configuratorhelper.cpp, logmanager.cpp (3×),
patternformatter.cpp, wdcappender.cpp, denyallfilter.h, nullappender.cpp
5. Replace recursive variadic templates with C++17 fold expressions:
- logger.h: 9 methods (debug, error, fatal, info, log, logWithLocation,
trace, warn + MessageLogger::log)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nstexpr, concepts 1. Structured bindings with asKeyValueRange() (C++17): - logmanager.cpp: QHash iteration in environment settings trace logging - xmllayout.cpp: QHash iteration in XML properties output 2. Init-statements in if/switch (C++17): - propertyconfigurator.cpp: qobject_cast with immediate if-check - telnetappender.cpp: nextPendingConnection() with null check - systemlogappender.cpp: RegisterEventSource() with null check 3. constexpr expansion: - binaryloggingevent.cpp: static const char[] -> constexpr char[] - dailyfileappender.cpp: static const char[] -> constexpr char[] 4. C++20 concept for LogStream::operator<< (logstream.h): - QTextStreamable concept constrains template parameter - Guarded with #ifdef __cpp_concepts for C++17 compatibility 5. Cleaned up log4qtsharedptr.h documentation comment Skipped (breaking API changes): - std::optional for bool* ok parameters (P3.6) - std::source_location for __FILE__/__LINE__ (P3.7) - QStringView for const QString& parameters (P3.5) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add opt-in std::source_location constructors and overloads alongside existing __FILE__/__LINE__/Q_FUNC_INFO-based API. All additions are guarded by __cpp_lib_source_location feature-test macro for backward compatibility with C++17 builds. Changes: - MessageContext: new constructor from std::source_location - MessageLogger: new constructor taking std::source_location - Logger::logWithLocation: new overload taking std::source_location with default parameter std::source_location::current() Existing macros (l4qFatal, l4qError, etc.) remain unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verifies that MessageContext constructed from std::source_location correctly captures file name, line number, and function name, and that these values propagate through LoggingEvent and PatternFormatter. Test is guarded with #ifdef __cpp_lib_source_location and uses QSKIP on compilers that lack support. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace const QString & with QStringView where the parameter is only inspected (compared, parsed) and never stored: - Level::fromString(QStringView) - pure string comparisons - LoggerRepository/Hierarchy::setThreshold(QStringView) - forwards to Level::fromString - PatternFormatter::parseIntegeoption(QStringView) - isEmpty/toInt only This avoids unnecessary QString copies when callers pass substrings or string views. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ctices This commit modernizes the entire CMake build system to follow modern CMake best practices and conventions. Key improvements: * Update minimum CMake version from 3.10 to 3.16 * Add project metadata (description, homepage, language) * Replace global add_definitions() with target-based compile definitions * Replace global include_directories() with target-based includes * Convert string-based compiler flags to list-based variables * Add Log4Qt::log4qt ALIAS target for better namespacing * Improve package configuration with configure_package_config_file() * Add automatic Qt dependency finding via find_dependency() * Modernize all if/endif syntax to lowercase without redundant arguments * Improve code formatting and consistency across all CMake files * Fix module include order (CMakePackageConfigHelpers before usage) Benefits: - Target-based build system prevents global pollution - Better dependency management with automatic Qt finding - Relocatable packages can be installed anywhere - Backward compatible with existing projects - Better IDE support and integration - Cleaner, more maintainable code Files modified: - CMakeLists.txt: Main project configuration - src/log4qt/CMakeLists.txt: Library target modernization - cmake/Log4QtConfig.cmake.in: Package config improvements - cmake/MacroEnsureOutOfSourceBuild.cmake: Syntax modernization - cmake/cmake_uninstall.cmake.in: Formatting improvements - CMAKE_MODERNIZATION_SUMMARY.md: Complete documentation of changes
- Remove legacy variable support (Log4Qt_LIBRARIES, Log4Qt_INCLUDE_DIRS) - Enforce modern CMake target usage (Log4Qt::log4qt only) - Update documentation to reflect clean modern interface - Simplify Log4QtConfig.cmake.in by removing compatibility layer BREAKING CHANGE: Projects using legacy variables must migrate to modern imported targets. Replace target_link_libraries with Log4Qt::log4qt.
- Remove all .pro and .pri files (17 files total) - Update Readme.md to document CMake as the only build system - Update appveyor.yml CI to use CMake instead of qmake - Project now exclusively uses CMake for building
# Conflicts: # appveyor.yml # log4qt.pro
- cache thread name in thread_local storage - call QThread::currentThread() only once per event
…erring stream creation
…bug, info, warn, etc.) to check isEnabledFor() before performing expensive string interpolation.
LE_COPY_MOVE fallback macro for Qt < 5.13 - Replace Q_DISABLE_COPY with Q_DISABLE_COPY_MOVE in DatabaseAppender, DatabaseLayout, TelnetAppender - Replace QLatin1String with u"..."_s string literals in hierarchy.cpp, properties.cpp - Replace QScopedPointer with std::unique_ptr in TTCCLayout - Use QString::fromLatin1() instead of QLatin1String(const char*) in ClassLogger Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ream, DailyFileAppender Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the poll-until-cached approach in setThreadNameToCurrent() with a QPropertyNotifier that sets a dirty flag when the thread's objectName changes. The cached name is re-read lazily on the next call, so thread renames are picked up immediately. The hex pointer fallback is also cached per-thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The cache window setting is written by setCacheWindow() and read concurrently from every logging thread. Using std::atomic with relaxed ordering ensures data-race-free access without adding synchronization overhead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
QEvent is not move-aware — the compiler-generated move ops fall back to copying QEvent's internal state (including the posted flag), which can cause double-deletion when the event dispatcher is involved. Removing them lets the implicitly-deleted move ops fall back to the explicitly defaulted copy operations, which is safe. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The notifier lambda captured a raw QThread* that could dangle if the QThread object is destroyed before thread-local storage cleanup. Use QPointer<QThread> so the callback safely no-ops when the QThread has already been deleted. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A race between checkEntryConditions (relaxed atomic reads of isActive/ isClosed) and closeInternal (which resets mpDispatcher under the lock) could allow append() to dereference a null mpDispatcher. Guard the postEvent call with a null check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The hex pointer fallback string was a static thread_local const, initialized once and never updated. If the QThread wrapper object changes (e.g. thread pool reuse), the cached address becomes stale. Refresh it alongside cachedName whenever the nameChanged flag is set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MessageContext::file and ::function are raw const char* initialized to nullptr by the default constructor. Appending nullptr to QString is undefined behaviour. Add null checks for FILENAME_CONVERTER, FUNCTIONNAME_CONVERTER, and LOCATION_CONVERTER. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The actual storage is file-scope statics in the .cpp file. The commented-out member declarations were leftover from a draft and misrepresent the class interface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
If currentMSecsSinceEpoch() is called from another translation unit's static initializer, the file-scope s_elapsedTimer may not have been started yet. Add an isValid() check to lazily start it on first use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the log level is disabled, no internal stream is allocated and all data streamed via operator<< is silently discarded. Document this on the constructor so maintainers understand the intentional behaviour. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove duplicated find_package and CMAKE_RUNTIME_OUTPUT_DIRECTORY calls that were left over from concatenating two separate CMakeLists.txt files. Both targets now share a single find_package and output dir. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A negative elapsedSinceLastUpdate (e.g. from timer restart) would prevent the cache from ever refreshing. Treat it as expired alongside the normal window check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mLevel is read from isEnabledFor/effectiveLevel on logging threads and written by setLevel during configuration. Make it std::atomic<Level> consistent with mAdditivity to eliminate the data race. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move all data members into a QSharedData-derived inner struct and store them via QSharedDataPointer. This enables copy-on-write semantics so that copying LoggingEvent (e.g. when posting across threads or storing in lists) only bumps a reference count instead of deep-copying all QString and QHash members. Add benchmark tests comparing the new implicitly shared implementation against the old direct-member layout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a JsonConfigurator class that reads JSON configuration files and flattens them into Properties for delegation to PropertyConfigurator. This enables `log4qt.json` as an alternative to `log4qt.properties` with zero duplication of parsing logic. Key features: - Nested JSON objects produce dot-separated property keys - @Class key maps to the parent object's class name - Booleans/numbers are stringified, variable substitution works - LogManager auto-discovery searches .json after .properties at each location for backward compatibility - File watching via configureAndWatch() is supported Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add XmlConfigurator that reads .xml config files, flattens them into Properties (nested elements become dot-separated keys, 'class' attribute sets the element's class name), and delegates to PropertyConfigurator. LogManager auto-initialization now searches for log4qt.xml after log4qt.json (.properties > .json > .xml priority). Includes real-world test cases for both JSON and XML configurators based on a production config with ConsoleAppender, DailyFileAppender, TTCCLayout with custom date format, and variable substitution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the legacy Log4j1-style property format (log4j.appender.X=ClassName, log4j.rootLogger=LEVEL, appenders) with a modern Log4j2-style structured format using explicit type keys and appender references: - PropertyConfigurator: new keys like appender.X.type, rootLogger.level, rootLogger.appenderRef.N.ref, logger.X.name/level/additivity - Global settings: reset, status, threshold, handleQtMessages (no log4j. prefix) - Factory: add short aliases (Console, File, PatternLayout, etc.) - JsonConfigurator: simple flattening without @Class handling - XmlConfigurator: skip root element name, flatten children directly, support attributes as child properties - Update all tests (JSON, XML, property configurator) for new format - Add dedicated PropertyConfigurator unit test suite (tests/propertytest) - Update example .properties file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Configuration.md documenting the Log4j2-style configuration format for all three file types (Properties, JSON, XML) with complete examples, reference tables for all built-in components, and usage instructions. Update ChangeLog.md with detailed v2.0.0 breaking changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Handle "INHERITED" level string explicitly in configureLoggers by setting Level::NULL_INT, since OptionConverter::toLevel does not recognize it and would fall back to DEBUG. - Only call removeAllAppenders() on root logger and named loggers when appenderRef keys are actually present, preventing unintended wipe of existing appenders during incremental reconfiguration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ngs docs - Warn when a filter is configured for a non-AppenderSkeleton appender - Rewrite XML testFlattenAttributes to actually use XML attributes - Clarify Log4j2-style flat key format in QSettings documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- CMakeLists.txt: replace Qt5/Qt6 dual detection with Qt6-only find_package, set minimum version to 6.4.0, update QT_DISABLE_DEPRECATED_BEFORE to 0x060400 - Readme.md: update minimum Qt version requirement to Qt 6.4 - ChangeLog.md: document breaking change in v2.0.0 - .github/copilot-instructions.md: add Copilot instructions for the repository Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # src/log4qt/binaryfileappender.cpp # src/log4qt/binaryfileappender.h # src/log4qt/binarylayout.cpp # src/log4qt/binarylayout.h # src/log4qt/binarylogger.cpp # src/log4qt/binarylogger.h # src/log4qt/binaryloggingevent.cpp # src/log4qt/binaryloggingevent.h # src/log4qt/binarylogstream.cpp # src/log4qt/binarylogstream.h # src/log4qt/binarytotextlayout.cpp # src/log4qt/binarytotextlayout.h # src/log4qt/binarywriterappender.cpp # src/log4qt/binarywriterappender.h # src/log4qt/helpers/binaryclasslogger.cpp # src/log4qt/helpers/binaryclasslogger.h # src/log4qt/rollingbinaryfileappender.cpp # src/log4qt/rollingbinaryfileappender.h # src/log4qt/varia/binaryeventfilter.cpp # src/log4qt/varia/binaryeventfilter.h
andibacher
marked this pull request as draft
March 24, 2026 09:59
Detect log4j. prefix keys and translate to Log4j2 format before parsing. Supports class-as-value appenders/layouts/filters, inline comma-separated root/logger values, category/rootCategory aliases, and additivity mapping. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added
log4qt.json).The JSON structure maps directly to flat property keys via dot-separated
nesting. Automatic initialization searches for
log4qt.jsonalongsidelog4qt.properties(.propertiestakes priority).log4qt.xml).Nested elements produce dot-separated keys; XML attributes are flattened
as child properties. Automatic initialization searches for
log4qt.xmlafter
log4qt.json(.properties>.json>.xmlpriority).Console,File,RollingFile,PatternLayout,LevelMatch) alongside theexisting
org.apache.log4j.*andLog4Qt::*names.tests/propertytest).Changed
supported. Use branch 1.6 / release 1.6.x for Qt 5.7+ support.
format. The
log4j.prefix is removed, appenders use explicittypekeys,and loggers use appender references instead of inline declarations:
log4j.rootLogger=ALL, console→rootLogger.level=ALL+rootLogger.appenderRef.0.ref=consolelog4j.appender.X=org.apache.log4j.ConsoleAppender→appender.X.type=Consolelog4j.appender.X.layout=org.apache.log4j.TTCCLayout→appender.X.layout.type=TTCCLayoutlog4j.logger.MyApp=ERROR, console→logger.MyApp.name=MyApp+logger.MyApp.level=ERROR+logger.MyApp.appenderRef.0.ref=consolelog4j.additivity.MyApp=false→logger.MyApp.additivity=falselog4j.reset→reset,log4j.Debug→status,log4j.threshold→threshold, etc.appender.X.filter.F.type=LevelMatchwith properties under the same prefix.
Improvements
Deprecated / Removed