Re: [devel] [PATCH 1/1] dtm: improve time accuracy in a trace record [#3144]

[Minh Hon Chau]

Hi aThanh,

The CreateLogEntry/... are added recently in LogTraceClient.h/cc, and 
the "client" you mean should have been calling the log/trace in 
logtrace.h, which are the OpenSAF services and agents. The real client 
should include the SAF headers in src/ais/include to use SAF services.

Do you see any use cases that "client" should include LogTraceClient.h 
to call CreateLogEntry without logtrace.h?

Thanks, Minh

On 6/2/20 2:05 pm, Thanh Nguyen wrote:
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