You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/10/19 10:11:56 UTC
[GitHub] [logging-log4cxx] swebb2066 opened a new pull request #73: Throughput tests
swebb2066 opened a new pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73
This PR provides a tool for measuring the cost of changes to the Logger and Hierarchy classes.
I have measured the cost of the std::mutex lock in Hierarchy::isDisabled as approx 1300 ns.
I will do a separate PR for Hierarchy::isDisabled if required.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #73: Throughput tests
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73#issuecomment-947277364
> It is a benchmarking thing. Your benchmarking tools look good. When are you thinking you will merge?
I don't have any timeframe for that at the moment. It is mostly so that I can find areas for improvement at the moment. There has been some talk on the development list about continuous performance testing for log4j2, so it would be nice to be able to piggyback off of that at some point.
> Have you run the benchmarks without the std::mutex lock in Hierarchy::isDisabled()? (I do not think that lock is necessary as it is only a read)
I have not. My concern with changing things like that is that we don't have great tests at the moment that test various multithreaded configurations(LOGCXX-534 [comes to mind](https://github.com/apache/logging-log4cxx/commit/a0a24987fd28b2bcd2a2628c949b01745851c634)).
If you want to discuss more on the performance aspects, that's probably better discussed on the mailing list. I'm working on some long-term issues at the moment, and performance is something that I'm looking to improve, if you're looking to help.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 commented on pull request #73: Throughput tests
Posted by GitBox <gi...@apache.org>.
swebb2066 commented on pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73#issuecomment-947167006
It is a benchmarking thing. Your benchmarking tools look good. When are you thinking you will merge?
Have you run the benchmarks without the std::mutex lock in Hierarchy::isDisabled()? (I do not think that lock is necessary as it is only a read)
________________________________
From: Robert Middleton ***@***.***>
Sent: Wednesday, October 20, 2021 9:33 AM
To: apache/logging-log4cxx ***@***.***>
Cc: Stephen Webb ***@***.***>; Author ***@***.***>
Subject: Re: [apache/logging-log4cxx] Throughput tests (PR #73)
Is the objective to have a unit test, or a benchmarking test? Those are two somewhat differing goals in my mind - a unit test makes sure that things are working, while a benchmarking test shows how fast under ideal circumstances. Doing this as a unit test doesn't make much sense to me, since it really doesn't test much in terms of a regression.
FWIW here's my repo with my benchmarking tools; at some point I do want to merge it in to do continuous benchmarking tests: https://github.com/rm5248/log4cxx-bench
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://github.com/apache/logging-log4cxx/pull/73#issuecomment-947155410>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADJDRC7R7Y7V773C6FCQA4LUHXW4NANCNFSM5GIYCHEA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #73: Throughput tests
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73#issuecomment-999254310
I've added my performance tests in a separate PR(#87). I think it is equivalent to this PR and covers more cases. If you are able to take a look at it and provide feedback, I'd appreciate it. I did add a test to check multithreaded performance when logging is disabled(based off of this PR), so it has been useful.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #73: Throughput tests
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73#issuecomment-947155410
Is the objective to have a unit test, or a benchmarking test? Those are two somewhat differing goals in my mind - a unit test makes sure that things are working, while a benchmarking test shows how fast under ideal circumstances. Doing this as a unit test doesn't make much sense to me, since it really doesn't test much in terms of a regression.
FWIW here's my repo with my benchmarking tools; at some point I do want to merge it in to do continuous benchmarking tests: https://github.com/rm5248/log4cxx-bench
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 commented on pull request #73: Throughput tests
Posted by GitBox <gi...@apache.org>.
swebb2066 commented on pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73#issuecomment-1001350144
This has been superceded by #87
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 closed pull request #73: Throughput tests
Posted by GitBox <gi...@apache.org>.
swebb2066 closed pull request #73:
URL: https://github.com/apache/logging-log4cxx/pull/73
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org