[GitHub] [logging-log4cxx] rm5248 edited a comment on pull request #87: Imported and slightly cleaned up throughput tests

[GitBox <gi...@apache.org>]

rm5248 edited a comment on pull request #87:
URL: https://github.com/apache/logging-log4cxx/pull/87#issuecomment-1000014049


   > 1.  Multi threaded logging to a single logger probably should be part of the unit tests rather than a throughput test. The mutex in log4cxx::Hierarchy (or AppenderAttachableImpl::appendLoopOnAppenders) effectively serialize the threads, so the results are not interesting. Also, in multi-thread applications, each thread is likely to use a different log4cxx::logger instance. I see the MultithreadTest unit test you added in March 2021 covers the single logger case.
   > 
   
   That is a good thought, I'll look into adding that test in the next few days.  I think it really depends on how you're logging as well, if you have a logger instance per-class or a logger instance per-thread(these are not mutually exclusive).  I do agree that there definitely needs to be more multithreaded unit tests to help identify issues with threading.
   
   Part of the performance testing is to also help us find areas where we can better parallelize the logging.  There are probably a number of places we can replace a `mutex` with a `shared_mutex`.
   
   >  2. It would be nice to be able to easily monitor changes in throughput in the future as changes (PRs) are proposed. I see in https://medium.com/nerd-for-tech/ci-build-performance-testing-with-github-action-e6b227097c83 there is already support available. Would you like me to spend some time looking at the options? How do you feel about introducing another dependency for CI benchmarking?
   
   If you could do that, that would be great!  Note that within the ASF repos, we can't use any actions except for the offical github actions, so it may need to be something custom.  At least saving the data from the run would at least be a step in the right direction.  As long as any options in CMake are configurable by the user so they don't need to run them by default, that would be great.


-- 
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