Re: [PR] Reduce the overhead of unconfigured loggers [logging-log4cxx]
swebb2066 commented on PR #351: URL: https://github.com/apache/logging-log4cxx/pull/351#issuecomment-1938092708 On Ubuntu (gcc 11), the `benchmark/tools/compare.py --no-color benchmarks /tmp/old.json /tmp/new.json` results are: | Benchmark | Time | CPU | Time Old | Time New | CPU Old | CPU New | | | | | | | | | | Testing disabled logging request | -0.0358 | -0.0354 | 0 | 0 | 0 | 0 | | Testing disabled logging request/threads:4 | -0.2218 | -0.2217 | 0 | 0 | 1 | 1 | | Logging 5 char string using MessageBuffer, pattern: %m%n | -0.0704 | -0.0703 | 311 | 289 | 311 | 289 | | Logging 5 char string using MessageBuffer, pattern: %m%n/threads:4 | -0.0560 | -0.0531 | 553 | 522 | 2172 | 2056 | | Logging 49 char string using MessageBuffer, pattern: %m%n | -0.0133 | -0.0133 | 318 | 314 | 318 | 314 | | Logging 49 char string using MessageBuffer, pattern: %m%n/threads:4 | -0.2431 | -0.2367 | 575 | 435 | 2262 | 1727 | | Logging int value using MessageBuffer, pattern: %m%n | -0.0474 | -0.0475 | 489 | 466 | 489 | 466 | | Logging int value using MessageBuffer, pattern: %m%n/threads:4 | -0.2029 | -0.2014 | 593 | 473 | 2345 | 1872 | | Logging int+float using MessageBuffer, pattern: %m%n | -0. | -0. | 941 | 941 | 941 | 941 | | Logging int+float using MessageBuffer, pattern: %m%n/threads:4 | -0.2770 | -0.2735 | 754 | 545 | 2997 | 2177 | | Logging int value using MessageBuffer, pattern: [%d] %m%n | -0.1074 | -0.1074 | 627 | 559 | 627 | 559 | | Logging int value using MessageBuffer, pattern: [%d] [%c] [%p] %m%n | -0.1052 | -0.1051 | 629 | 563 | 629 | 563 | | Logging 49 char string using FMT, pattern: %m%n | -0.0333 | -0.0333 | 293 | 283 | 293 | 283 | | Logging 49 char string using FMT, pattern: %m%n/threads:4 | -0.1931 | -0.1891 | 537 | 433 | 2114 | 1714 | | Logging int value using FMT, pattern: %m%n | -0.0156 | -0.0156 | 315 | 310 | 315 | 310 | | Logging int value using FMT, pattern: %m%n/threads:4 | -0.1616 | -0.1590 | 548 | 459 | 2162 | 1818 | | Logging int+float using FMT, pattern: %m%n | -0.0558 | -0.0559 | 468 | 442 | 468 | 442 | | Logging int+float using FMT, pattern: %m%n/threads:4 | +0.0794 | +0.0839 | 429 | 463 | 1700 | 1843 | | Async, int value using MessageBuffer, pattern: %m%n | -0.0463 | -0.0463 | 493 | 471 | 493 | 471 | | Async, int value using MessageBuffer, pattern: %m%n/threads:4 | -0.3025 | -0.2992 | 680 | 474 | 2683 | 1880 | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Reduce the overhead of unconfigured loggers [logging-log4cxx]
swebb2066 commented on PR #351: URL: https://github.com/apache/logging-log4cxx/pull/351#issuecomment-1938079026 > What's the reasoning behind the lazy initialization of m_priv? It makes the code harder to read IMO since you now have an extra check to see if it is valid, instead of just knowing that it always exists. Less memory and processor overhead (see the above benchmarks). An application will create many loggers, but few (usually only the root logger) has attached appenders. Lazy initialization reduces memory alloc's by avoiding allocating `AppenderAttachableImpl::priv_data` (and initializing a std::mutex + std::vector) for all those loggers that do not have attached appenders. Throughput is improved because `appendLoopOnAppenders` does not acquire a lock for each decendent logger in the path to the root logger. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Reduce the overhead of unconfigured loggers [logging-log4cxx]
rm5248 commented on PR #351: URL: https://github.com/apache/logging-log4cxx/pull/351#issuecomment-1938068864 What's the reasoning behind the lazy initialization of `m_priv`? It makes the code harder to read IMO since you now have an extra check to see if it is valid, instead of just knowing that it always exists. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Reduce the overhead of unconfigured loggers [logging-log4cxx]
swebb2066 commented on PR #351: URL: https://github.com/apache/logging-log4cxx/pull/351#issuecomment-1937974342 On Windows, the benchmark comparisons using a lock (old) and without locking (new) in AppenderAttachableImpl::appendLoopOnAppenders are: | Benchmark | Time | CPU | Time Old | Time New | CPU Old | CPU New | | | | | | | | | | Testing disabled logging request | +0.0034 | +0.1569 | 4 | 4 | 1 | 1 | | Testing disabled logging request/threads:4 | -0.2516 | -0.4159 | 1 | 1 | 4 | 2 | | Logging 5 char string using MessageBuffer, pattern: %m%n | -0.1902 | -0.2120 | 738 | 597 | 443 | 349 | | Logging 5 char string using MessageBuffer, pattern: %m%n/threads:4 | +0.0410 | -0.5887 | 437 | 455 | 1289 | 530 | | Logging 49 char string using MessageBuffer, pattern: %m%n | -0.0180 | -0.1123 | 752 | 739 | 481 | 427 | | Logging 49 char string using MessageBuffer, pattern: %m%n/threads:4 | -0.0485 | -0.3203 | 462 | 439 | 1242 | 844 | | Logging int value using MessageBuffer, pattern: %m%n | -0.0725 | -0.1875 | 1756 | 1629 | 1099 | 893 | | Logging int value using MessageBuffer, pattern: %m%n/threads:4 | -0.1219 | -0.3980 | 740 | 650 | 1744 | 1050 | | Logging int+float using MessageBuffer, pattern: %m%n | -0.1529 | -0.3460 | 3535 | 2995 | 2288 | 1496 | | Logging int+float using MessageBuffer, pattern: %m%n/threads:4 | -0.1841 | -0.3902 | 1387 | 1132 | 3139 | 1914 | | Logging int value using MessageBuffer, pattern: [%d] %m%n | -0.1133 | -0.2317 | 1868 | 1656 | 1144 | 879 | | Logging int value using MessageBuffer, pattern: [%d] [%c] [%p] %m%n | -0.0990 | -0.2480 | 1876 | 1690 | 1308 | 984 | | Logging 49 char string using FMT, pattern: %m%n | -0.0972 | -0.2988 | 796 | 719 | 572 | 401 | | Logging 49 char string using FMT, pattern: %m%n/threads:4 | -0.1717 | -0.4716 | 489 | 405 | 1135 | 600 | | Logging int value using FMT, pattern: %m%n | -0.0587 | -0.0932 | 712 | 670 | 460 | 417 | | Logging int value using FMT, pattern: %m%n/threads:4 | -0.1114 | -0.3957 | 458 | 407 | 1062 | 642 | | Logging int+float using FMT, pattern: %m%n | -0.1281 | -0.1353 | 1132 | 987 | 663 | 573 | | Logging int+float using FMT, pattern: %m%n/threads:4 | -0.1281 | -0.5153 | 544 | 475 | 1662 | 806 | | Async, int value using MessageBuffer, pattern: %m%n | -0.1272 | -0.2650 | 1824 | 1592 | 1224 | 900 | | Async, int value using MessageBuffer, pattern: %m%n/threads:4 | -0.0474 | -0.3363 | 745 | 709 | 1883 | 1250 | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Reduce the overhead of unconfigured loggers [logging-log4cxx]
swebb2066 opened a new pull request, #351: URL: https://github.com/apache/logging-log4cxx/pull/351 AppenderAttachableImpl::appendLoopOnAppenders does not need to lock a mutex for any Logger in the path to the root Logger which has no appenders attached (This partly addressed #347). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Logging context convenience functions for coroutines [logging-log4j-kotlin]
ppkarwasz commented on PR #65: URL: https://github.com/apache/logging-log4j-kotlin/pull/65#issuecomment-1937852187 > @rocketraman, we (almost) always use `Squash and Commit` in Log4j. You could have done the same for this PR. That is, your PR would appear as a single commit in `main`. Doesn't that address your concern? That is really a question of commit style. Personally I prefer force pushes like @rocketraman and I almost never make merge commits. Unless the number of commits is high or commits have no well-defined purposes (e.g. a contributor changes his contribution back and forth) I don't squash either. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Logging context convenience functions for coroutines [logging-log4j-kotlin]
vy commented on PR #65: URL: https://github.com/apache/logging-log4j-kotlin/pull/65#issuecomment-1937845625 > My preference is generally to sacrifice the GitHub PR cleanliness in favor of a clean commit history, as the former is transient and the latter is forever. @rocketraman, we (almost) always use `Squash and Commit` in Log4j. You could have done the same for this PR. That is, your PR would appear as a single commit in `main`. Doesn't that address your concern? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (LOGCXX-576) Logging DOAP file has an error
[
https://issues.apache.org/jira/browse/LOGCXX-576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17816443#comment-17816443
]
Robert Middleton commented on LOGCXX-576:
-
Line 26 has:
{code:java}
Apache log4cxx {code}
So I'm not sure where this error is coming from. What is generating this error
and how can I test it?
> Logging DOAP file has an error
> --
>
> Key: LOGCXX-576
> URL: https://issues.apache.org/jira/browse/LOGCXX-576
> Project: Log4cxx
> Issue Type: Bug
>Reporter: Claude Warren
>Priority: Minor
>
> The DOAP file [1] as listed in [2] has the error:
> [line: 26, col: 4 ] \{E201} The attributes on this property element, are not
> permitted with any content; expecting end element tag.
> [1]
> https://gitbox.apache.org/repos/asf?p=logging-log4cxx.git;a=blob_plain;f=doap_log4cxx.rdf;hb=HEAD
> [2]
> https://svn.apache.org/repos/asf/comdev/projects.apache.org/trunk/data/projects.xml
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (LOGCXX-576) Logging DOAP file has an error
[
https://issues.apache.org/jira/browse/LOGCXX-576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17816430#comment-17816430
]
Gary D. Gregory commented on LOGCXX-576:
Moving this issue to the LOGCXX Jira project.
> Logging DOAP file has an error
> --
>
> Key: LOGCXX-576
> URL: https://issues.apache.org/jira/browse/LOGCXX-576
> Project: Log4cxx
> Issue Type: Bug
>Reporter: Claude Warren
>Priority: Minor
>
> The DOAP file [1] as listed in [2] has the error:
> [line: 26, col: 4 ] \{E201} The attributes on this property element, are not
> permitted with any content; expecting end element tag.
> [1]
> https://gitbox.apache.org/repos/asf?p=logging-log4cxx.git;a=blob_plain;f=doap_log4cxx.rdf;hb=HEAD
> [2]
> https://svn.apache.org/repos/asf/comdev/projects.apache.org/trunk/data/projects.xml
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Moved] (LOGCXX-576) Logging DOAP file has an error
[
https://issues.apache.org/jira/browse/LOGCXX-576?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gary D. Gregory moved LOGGING-184 to LOGCXX-576:
Key: LOGCXX-576 (was: LOGGING-184)
Affects Version/s: (was: 1.2)
Project: Log4cxx (was: Commons Logging)
> Logging DOAP file has an error
> --
>
> Key: LOGCXX-576
> URL: https://issues.apache.org/jira/browse/LOGCXX-576
> Project: Log4cxx
> Issue Type: Bug
>Reporter: Claude Warren
>Priority: Minor
>
> The DOAP file [1] as listed in [2] has the error:
> [line: 26, col: 4 ] \{E201} The attributes on this property element, are not
> permitted with any content; expecting end element tag.
> [1]
> https://gitbox.apache.org/repos/asf?p=logging-log4cxx.git;a=blob_plain;f=doap_log4cxx.rdf;hb=HEAD
> [2]
> https://svn.apache.org/repos/asf/comdev/projects.apache.org/trunk/data/projects.xml
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
Re: [PR] reorder LOG4CXX_NS cmake variable definition/usage [logging-log4cxx]
vopl commented on PR #350: URL: https://github.com/apache/logging-log4cxx/pull/350#issuecomment-1937693885 https://github.com/apache/logging-log4cxx/issues/349 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] CMake build failure with MultiprocessRollingFileAppender [logging-log4cxx]
swebb2066 commented on issue #349: URL: https://github.com/apache/logging-log4cxx/issues/349#issuecomment-1937609722 This is a bug. The LOG4CXX_NS cmake variable is not available when `src/cmake/boost-fallback/boost-std-configuration.h.cmake` is being processed as it is not exported from `src\main\include\CMakeLists.txt` to the parent context. I think the following statement should be moved from `src\main\include\CMakeLists.txt` to the top level CMakeLists.txt. `set(LOG4CXX_NS "log4cxx" CACHE STRING "Root namespace name")` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] CMake build failure with MultiprocessRollingFileAppender [logging-log4cxx]
vopl commented on issue #349: URL: https://github.com/apache/logging-log4cxx/issues/349#issuecomment-1937499893 LOG4CXX_NS is a cached variable with the default value "log4cxx", declared here https://github.com/apache/logging-log4cxx/blob/master/src/main/include/CMakeLists.txt#L82. The problem is the empty LOG4CXX_NS value in your case, but an empty value is impossible with regular project usage. Please, try to clear CMake cache and reconfigure from scratch, the problem should gone. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] CMake build failure with MultiprocessRollingFileAppender [logging-log4cxx]
perhapsmaple opened a new issue, #349:
URL: https://github.com/apache/logging-log4cxx/issues/349
Hi, I am trying to build log4cxx with the MultiprocessRollingFileAppender
using CMake with the following command:
```
cmake -B build -S . -DLOG4CXX_MULTIPROCESS_ROLLING_FILE_APPENDER=ON
-DCMAKE_CXX_STANDARD=17
```
When I build the project, I get the following error:
```
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:
In member function ‘const string
log4cxx::rolling::TimeBasedRollingPolicy::createFile(const string&, const
string&, log4cxx::helpers::Pool&)’:
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:181:21:
error: ‘log4cxx::filesystem’ has not been declared
181 | LOG4CXX_NS::filesystem::path path(fileName);
| ^~
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:182:35:
error: ‘path’ was not declared in this scope
182 | std::string newFilename = path.filename().string() + szUid +
suffix;
| ^~~~
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:182:35:
note: suggested alternatives:
In file included from /usr/include/c++/11/filesystem:45,
from
/home/harish/build/original-log4cxx/build/src/main/include/log4cxx/private/boost-std-configuration.h:9,
from
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:31:
/usr/include/c++/11/bits/fs_path.h:248:9: note:
‘std::filesystem::__cxx11::path’
248 | class path
| ^~~~
In file included from
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:31:
/home/harish/build/original-log4cxx/build/src/main/include/log4cxx/private/boost-std-configuration.h:12:35:
note: ‘{anonymous}::filesystem::path’
12 | typedef std::filesystem::path path;
| ^~~~
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:183:21:
error: ‘log4cxx::filesystem’ has not been declared
183 | LOG4CXX_NS::filesystem::path retval = path.parent_path() /
newFilename;
| ^~
/home/harish/build/original-log4cxx/src/main/cpp/timebasedrollingpolicy.cpp:184:16:
error: ‘retval’ was not declared in this scope
184 | return retval.string();
|^~
gmake[2]: *** [src/main/cpp/CMakeFiles/log4cxx.dir/build.make:1868:
src/main/cpp/CMakeFiles/log4cxx.dir/timebasedrollingpolicy.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1366:
src/main/cpp/CMakeFiles/log4cxx.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
```
After scratching my head for a bit, I found the boost-std-configuration.h
file inside my cmake build folder:
```
#ifndef BOOST_STD_CONFIGURATION_H
#define BOOST_STD_CONFIGURATION_H
#define STD_FILESYSTEM_FOUND 1
#define Boost_FILESYSTEM_FOUND 0
#define STD_EXPERIMENTAL_FILESYSTEM_FOUND 1
#if STD_FILESYSTEM_FOUND
#include
namespace {
namespace filesystem {
typedef std::filesystem::path path;
}
}
#elif STD_EXPERIMENTAL_FILESYSTEM_FOUND
#include
namespace {
namespace filesystem {
typedef std::experimental::filesystem::path path;
}
}
#elif Boost_FILESYSTEM_FOUND
#include
namespace {
namespace filesystem {
typedef boost::filesystem::path path;
}
}
#endif
#endif /* BOOST_STD_CONFIGURATION_H */
```
which is generated by this:
```
configure_file(${LOG4CXX_SOURCE_DIR}/src/cmake/boost-fallback/boost-std-configuration.h.cmake
${CMAKE_CURRENT_BINARY_DIR}/log4cxx/private/boost-std-configuration.h
```
That value is not set and comes up empty when trying to print with:
```message (NAMESPACE=${NAMESPACE_ALIAS})```
gives the following in the output:```NAMESPACE=```
The build errors do go away when built with the CMake option
```-DLOG4CXX_NS="log4cxx"```
I'm happy to help with any further investigations.
Built latest log4cxx with ```gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0```
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
