Profiling output respects loglevel. #986
Conversation
|
I'd argue that Profiler output should not respect I think a better approach to the problem is to make "Profiler started" a log message, which it essentially is, with an exception of Java API, where this message is a valid response returned from |
|
When testing, please verify all 3 ways to run profiler:
|
5cb4148 to
7b448d7
Compare
|
Simplified the approach, introduced a new Tested as an agent, asprof and Java API. |
9203db6 to
3cd9fc8
Compare
|
(rebased) |
3cd9fc8 to
6bdcbbd
Compare
src/log.cpp
Outdated
| LogLevel Log::_level = LOG_INFO; | ||
|
|
||
| int Log::fd() { | ||
| return _file ? fileno(_file) : 0; |
There was a problem hiding this comment.
_file must not be NULL, I think.
src/writer.h
Outdated
| int _fd; | ||
| char* _buf; | ||
| size_t _size; | ||
| bool _keepOpen = false; |
There was a problem hiding this comment.
This will issue a compiler warning with older gcc.
We should migrate to a newer C++ standard first, but that's a separate story.
src/writer.h
Outdated
| class LogWriter : public FileWriter { | ||
| LogLevel _logLevel; | ||
| public: | ||
| LogWriter(LogLevel logLevel = LOG_INFO) : FileWriter(Log::fd(), true) { |
There was a problem hiding this comment.
This creates an implicit coupling with Log::_file, which lives its own life.
Currently, it's not a problem, since Log is always closed after the output file, but this dependency is fragile.
Alternatively, LogWriter could call Log::writeRaw or something.
"Profiling started" and other messages are now INFO loglevel and will respect the current loglevel.
6bdcbbd to
7583ccc
Compare
|
After this change, "Profiling started" message goes to I'm more are more inclined to believe this message is redundant and can be removed entirely except for the Java API call. |
Description
"Profiling started" and other messages are now INFO loglevel and will respect the current loglevel.
Related issues
#960
Motivation and context
All messages should respect loglevel, "Profiling started" etc. messages did not.
How has this been tested?
With different
loglevelparameters:By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.