Hello Minh, 1) For LogInternal(..), there is only one version. The new replaced the old. This is private method, thus it can be safely replaced. 2) For CreateLogEntry(...) which is a public method. For compatibility reasons, I keep the old method and create the new method. I do not know if there is any client code of opensaf out there calling CreateLogEntry. If it is known that there is no client code, I will remove the old method.
3) For CreateLogEntryInternal(..), I also keep two versions corresponding to two versions of calling methods CreateLogEntry(..). I will remove the old version of (2) and (3) if it is confirmed that there is no client code calling CreateLogEntry(..). Best Regards, Thanh -----Original Message----- From: Minh Hon Chau [mailto:minh.c...@dektech.com.au] Sent: Thursday, 6 February 2020 12:48 PM To: Thanh Nguyen; peter.mcint...@dektech.com.au Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1/1] dtm: improve time accuracy in a trace record [#3144] Hi aThanh, The patch adds a new pair of CreateLogEntry/CreateLogEntryInternal with one extra parameter. If the old one (within 3 parameters) is not being used anywhere else, we can delete them. Thanks Minh On 24/1/20 11:34 am, Thanh Nguyen wrote: > In the trace record the time value is generated > after acquiring the mutex. The time accuracy is improved > when generated before seizing the mutext. > --- > src/base/logtrace.cc | 2 +- > src/base/logtrace_client.cc | 18 +++++++++++++----- > src/base/logtrace_client.h | 13 ++++++++++--- > 3 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc > index 8908c1ff3..9822879ab 100644 > --- a/src/base/logtrace.cc > +++ b/src/base/logtrace.cc > @@ -97,7 +97,7 @@ void trace_output(const char *file, unsigned line, unsigned > priority, > if (!entry) { > entry = gl_local_thread_trace->CreateLogEntry( > static_cast<base::LogMessage::Severity>(priority), > - preamble, ap); > + base::ReadRealtimeClock(), preamble, ap); > } > gl_thread_buffer.WriteToBuffer(entry); > } > diff --git a/src/base/logtrace_client.cc b/src/base/logtrace_client.cc > index e22112a43..484bd17e5 100644 > --- a/src/base/logtrace_client.cc > +++ b/src/base/logtrace_client.cc > @@ -96,19 +96,26 @@ const char* LogTraceClient::Log(LogTraceClient* tracelog, > const char* LogTraceClient::Log(base::LogMessage::Severity severity, > const char *fmt, va_list ap) { > if (log_socket_ != nullptr && log_mutex_ != nullptr) { > - return LogInternal(severity, fmt, ap); > + return LogInternal(severity, base::ReadRealtimeClock(), fmt, ap); > } > return nullptr; > } > > const char* LogTraceClient::LogInternal(base::LogMessage::Severity severity, > - const char *fmt, va_list ap) { > + timespec time_spec, const char *fmt, va_list ap) { > base::Lock lock(*log_mutex_); > - CreateLogEntryInternal(severity, fmt, ap); > + CreateLogEntryInternal(severity, time_spec, fmt, ap); > log_socket_->Send(buffer_.data(), buffer_.size()); > return buffer_.data(); > } > > +const char* LogTraceClient::CreateLogEntry(base::LogMessage::Severity > severity, > + timespec time_spec, const char *fmt, va_list ap) { > + base::Lock lock(*log_mutex_); > + return CreateLogEntryInternal(severity, time_spec, fmt, ap); > +} > + > +// This is original > const char* LogTraceClient::CreateLogEntry(base::LogMessage::Severity > severity, > const char *fmt, va_list ap) { > base::Lock lock(*log_mutex_); > @@ -116,12 +123,13 @@ const char* > LogTraceClient::CreateLogEntry(base::LogMessage::Severity severity, > } > > const char* LogTraceClient::CreateLogEntryInternal( > - base::LogMessage::Severity severity, const char *fmt, va_list ap) { > + base::LogMessage::Severity severity, timespec time_spec, > + const char *fmt, va_list ap) { > uint32_t id = sequence_id_; > sequence_id_ = id < kMaxSequenceId ? id + 1 : 1; > buffer_.clear(); > base::LogMessage::Write( > - base::LogMessage::Facility::kLocal1, severity, > base::ReadRealtimeClock(), > + base::LogMessage::Facility::kLocal1, severity, time_spec, > fqdn_, app_name_, proc_id_, msg_id_, > {{base::LogMessage::SdName{"meta"}, > {base::LogMessage::Parameter{base::LogMessage::SdName{"sequenceId"}, > diff --git a/src/base/logtrace_client.h b/src/base/logtrace_client.h > index 5b165e528..29aa79b95 100644 > --- a/src/base/logtrace_client.h > +++ b/src/base/logtrace_client.h > @@ -45,6 +45,8 @@ class LogTraceClient { > va_list ap); > const char* CreateLogEntry(base::LogMessage::Severity severity, > const char *fmt, va_list ap); > + const char* CreateLogEntry(base::LogMessage::Severity severity, > + timespec time_spec, const char *fmt, va_list ap); > void AddExternalBuffer(int64_t tid, LogTraceBuffer* buffer); > void RemoveExternalBuffer(int64_t tid); > void RequestFlushExternalBuffer(); > @@ -56,10 +58,15 @@ class LogTraceClient { > > private: > bool Init(const char *msg_id, WriteMode mode); > - const char* LogInternal(base::LogMessage::Severity severity, const char > *fmt, > - va_list ap); > + > + const char* LogInternal(base::LogMessage::Severity severity, > + timespec time_spec, const char *fmt, va_list ap); > const char* CreateLogEntryInternal(base::LogMessage::Severity severity, > - const char *fmt, va_list ap); > + timespec time_spec, const char *fmt, va_list ap); > + inline const char* CreateLogEntryInternal( > + base::LogMessage::Severity severity, const char *fmt, va_list ap) { > + return CreateLogEntryInternal(severity, base::ReadRealtimeClock(), fmt, > ap); > + } > static constexpr const uint32_t kMaxSequenceId = uint32_t{0x7fffffff}; > base::LogMessage::HostName fqdn_{""}; > base::LogMessage::AppName app_name_{""}; _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
