Remove TraceImpl logging thread.
Simplifies TraceImpl significantly. Chromium uses trace callbacks and
logs directly through the trace callback interface. Added slowdowns when
logging to file are expected to only affect test targets.
BUG=
R=andresp@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/34329004
Cr-Commit-Position: refs/heads/master@{#8529}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8529 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/build/common.gypi b/webrtc/build/common.gypi
index e680ba0..1472ebf 100644
--- a/webrtc/build/common.gypi
+++ b/webrtc/build/common.gypi
@@ -127,10 +127,6 @@
# enable schannel on windows.
'use_legacy_ssl_defaults%': 0,
- # Directly call the trace callback instead of passing it to a logging
- # thread. Used for components that provide their own threaded logging.
- 'rtc_use_direct_trace%': 0,
-
'conditions': [
['build_with_chromium==1', {
# Exclude pulse audio on Chromium since its prerequisites don't require
diff --git a/webrtc/build/webrtc.gni b/webrtc/build/webrtc.gni
index e128c51..7577589 100644
--- a/webrtc/build/webrtc.gni
+++ b/webrtc/build/webrtc.gni
@@ -67,10 +67,6 @@
# https://gcc.gnu.org/wiki/LinkTimeOptimization
rtc_use_lto = false
- # Directly call the trace callback instead of passing it to a logging
- # thread. Used for components that provide their own threaded logging.
- rtc_use_direct_trace = false
-
if (build_with_chromium) {
# Exclude pulse audio on Chromium since its prerequisites don't require
# pulse audio.
diff --git a/webrtc/system_wrappers/BUILD.gn b/webrtc/system_wrappers/BUILD.gn
index 208c9ef..a641d24 100644
--- a/webrtc/system_wrappers/BUILD.gn
+++ b/webrtc/system_wrappers/BUILD.gn
@@ -115,10 +115,6 @@
libs = []
deps = [ "..:webrtc_common" ]
- if (rtc_use_direct_trace) {
- defines += [ "WEBRTC_DIRECT_TRACE" ]
- }
-
if (is_android) {
sources += [
"interface/logcat_trace_context.h",
diff --git a/webrtc/system_wrappers/source/trace_impl.cc b/webrtc/system_wrappers/source/trace_impl.cc
index 28de280..c41159d 100644
--- a/webrtc/system_wrappers/source/trace_impl.cc
+++ b/webrtc/system_wrappers/source/trace_impl.cc
@@ -21,8 +21,6 @@
#include "webrtc/system_wrappers/source/trace_posix.h"
#endif // _WIN32
-#include "webrtc/system_wrappers/interface/sleep.h"
-
#define KEY_LEN_CHARS 31
#ifdef _WIN32
@@ -65,68 +63,15 @@
}
TraceImpl::TraceImpl()
- : critsect_interface_(CriticalSectionWrapper::CreateCriticalSection()),
- callback_(NULL),
+ : callback_(NULL),
row_count_text_(0),
file_count_text_(0),
- trace_file_(*FileWrapper::Create()),
- thread_(*ThreadWrapper::CreateThread(TraceImpl::Run, this,
- kHighestPriority, "Trace")),
- event_(*EventWrapper::Create()),
- critsect_array_(CriticalSectionWrapper::CreateCriticalSection()),
- next_free_idx_(),
- level_(),
- length_(),
- message_queue_(),
- active_queue_(0) {
- next_free_idx_[0] = 0;
- next_free_idx_[1] = 0;
-
- unsigned int tid = 0;
- thread_.Start(tid);
-
- for (int m = 0; m < WEBRTC_TRACE_NUM_ARRAY; ++m) {
- for (int n = 0; n < WEBRTC_TRACE_MAX_QUEUE; ++n) {
- message_queue_[m][n] = new
- char[WEBRTC_TRACE_MAX_MESSAGE_SIZE];
- }
- }
-}
-
-bool TraceImpl::StopThread() {
- // Release the worker thread so that it can flush any lingering messages.
- event_.Set();
-
- // Allow 10 ms for pending messages to be flushed out.
- // TODO(hellner): why not use condition variables to do this? Or let the
- // worker thread die and let this thread flush remaining
- // messages?
- SleepMs(10);
-
- // Make sure the thread finishes as quickly as possible (instead of having
- // to wait for the timeout).
- event_.Set();
- bool stopped = thread_.Stop();
-
- CriticalSectionScoped lock(critsect_interface_);
- trace_file_.Flush();
- trace_file_.CloseFile();
- return stopped;
+ trace_file_(FileWrapper::Create()) {
}
TraceImpl::~TraceImpl() {
- StopThread();
- delete &event_;
- delete &trace_file_;
- delete &thread_;
- delete critsect_interface_;
- delete critsect_array_;
-
- for (int m = 0; m < WEBRTC_TRACE_NUM_ARRAY; ++m) {
- for (int n = 0; n < WEBRTC_TRACE_MAX_QUEUE; ++n) {
- delete [] message_queue_[m][n];
- }
- }
+ trace_file_->Flush();
+ trace_file_->CloseFile();
}
int32_t TraceImpl::AddThreadId(char* trace_message) const {
@@ -338,10 +283,10 @@
int32_t TraceImpl::SetTraceFileImpl(const char* file_name_utf8,
const bool add_file_counter) {
- CriticalSectionScoped lock(critsect_interface_);
+ rtc::CritScope lock(&crit_);
- trace_file_.Flush();
- trace_file_.CloseFile();
+ trace_file_->Flush();
+ trace_file_->CloseFile();
if (file_name_utf8) {
if (add_file_counter) {
@@ -350,13 +295,13 @@
char file_name_with_counter_utf8[FileWrapper::kMaxFileNameSize];
CreateFileName(file_name_utf8, file_name_with_counter_utf8,
file_count_text_);
- if (trace_file_.OpenFile(file_name_with_counter_utf8, false, false,
+ if (trace_file_->OpenFile(file_name_with_counter_utf8, false, false,
true) == -1) {
return -1;
}
} else {
file_count_text_ = 0;
- if (trace_file_.OpenFile(file_name_utf8, false, false, true) == -1) {
+ if (trace_file_->OpenFile(file_name_utf8, false, false, true) == -1) {
return -1;
}
}
@@ -367,12 +312,12 @@
int32_t TraceImpl::TraceFileImpl(
char file_name_utf8[FileWrapper::kMaxFileNameSize]) {
- CriticalSectionScoped lock(critsect_interface_);
- return trace_file_.FileName(file_name_utf8, FileWrapper::kMaxFileNameSize);
+ rtc::CritScope lock(&crit_);
+ return trace_file_->FileName(file_name_utf8, FileWrapper::kMaxFileNameSize);
}
int32_t TraceImpl::SetTraceCallbackImpl(TraceCallback* callback) {
- CriticalSectionScoped lock(critsect_interface_);
+ rtc::CritScope lock(&crit_);
callback_ = callback;
return 0;
}
@@ -412,204 +357,97 @@
const char trace_message[WEBRTC_TRACE_MAX_MESSAGE_SIZE],
const uint16_t length,
const TraceLevel level) {
-// NOTE(andresp): Enabled externally.
-#ifdef WEBRTC_DIRECT_TRACE
- if (callback_) {
+ rtc::CritScope lock(&crit_);
+ if (callback_)
callback_->Print(level, trace_message, length);
- }
- return;
-#endif
-
- CriticalSectionScoped lock(critsect_array_);
-
- if (next_free_idx_[active_queue_] >= WEBRTC_TRACE_MAX_QUEUE) {
- if (!trace_file_.Open() && !callback_) {
- // Keep at least the last 1/4 of old messages when not logging.
- // TODO(hellner): isn't this redundant. The user will make it known
- // when to start logging. Why keep messages before
- // that?
- for (int n = 0; n < WEBRTC_TRACE_MAX_QUEUE / 4; ++n) {
- const int last_quarter_offset = (3 * WEBRTC_TRACE_MAX_QUEUE / 4);
- memcpy(message_queue_[active_queue_][n],
- message_queue_[active_queue_][n + last_quarter_offset],
- WEBRTC_TRACE_MAX_MESSAGE_SIZE);
- }
- next_free_idx_[active_queue_] = WEBRTC_TRACE_MAX_QUEUE / 4;
- } else {
- // More messages are being written than there is room for in the
- // buffer. Drop any new messages.
- // TODO(hellner): its probably better to drop old messages instead
- // of new ones. One step further: if this happens
- // it's due to writing faster than what can be
- // processed. Maybe modify the filter at this point.
- // E.g. turn of STREAM.
- return;
- }
- }
-
- uint16_t idx = next_free_idx_[active_queue_];
- next_free_idx_[active_queue_]++;
-
- level_[active_queue_][idx] = level;
- length_[active_queue_][idx] = length;
- memcpy(message_queue_[active_queue_][idx], trace_message, length);
-
- if (next_free_idx_[active_queue_] == WEBRTC_TRACE_MAX_QUEUE - 1) {
- // Logging more messages than can be worked off. Log a warning.
- const char warning_msg[] = "WARNING MISSING TRACE MESSAGES\n";
- level_[active_queue_][next_free_idx_[active_queue_]] = kTraceWarning;
- length_[active_queue_][next_free_idx_[active_queue_]] = strlen(warning_msg);
- memcpy(message_queue_[active_queue_][next_free_idx_[active_queue_]],
- warning_msg, strlen(warning_msg));
- next_free_idx_[active_queue_]++;
- }
+ WriteToFile(trace_message, length);
}
-bool TraceImpl::Run(void* obj) {
- return static_cast<TraceImpl*>(obj)->Process();
-}
-
-bool TraceImpl::Process() {
- if (event_.Wait(1000) == kEventSignaled) {
- // This slightly odd construction is to avoid locking |critsect_interface_|
- // while calling WriteToFile() since it's locked inside the function.
- critsect_interface_->Enter();
- bool write_to_file = trace_file_.Open() || callback_;
- critsect_interface_->Leave();
- if (write_to_file) {
- WriteToFile();
- }
- } else {
- CriticalSectionScoped lock(critsect_interface_);
- trace_file_.Flush();
- }
- return true;
-}
-
-void TraceImpl::WriteToFile() {
- uint8_t local_queue_active = 0;
- uint16_t local_next_free_idx = 0;
-
- // There are two buffers. One for reading (for writing to file) and one for
- // writing (for storing new messages). Let new messages be posted to the
- // unused buffer so that the current buffer can be flushed safely.
- {
- CriticalSectionScoped lock(critsect_array_);
- local_next_free_idx = next_free_idx_[active_queue_];
- next_free_idx_[active_queue_] = 0;
- local_queue_active = active_queue_;
- if (active_queue_ == 0) {
- active_queue_ = 1;
- } else {
- active_queue_ = 0;
- }
- }
- if (local_next_free_idx == 0) {
+void TraceImpl::WriteToFile(const char* msg, uint16_t length) {
+ if (!trace_file_->Open())
return;
- }
- CriticalSectionScoped lock(critsect_interface_);
+ if (row_count_text_ > WEBRTC_TRACE_MAX_FILE_SIZE) {
+ // wrap file
+ row_count_text_ = 0;
+ trace_file_->Flush();
- for (uint16_t idx = 0; idx < local_next_free_idx; ++idx) {
- TraceLevel local_level = level_[local_queue_active][idx];
- if (callback_) {
- callback_->Print(local_level, message_queue_[local_queue_active][idx],
- length_[local_queue_active][idx]);
+ if (file_count_text_ == 0) {
+ trace_file_->Rewind();
+ } else {
+ char old_file_name[FileWrapper::kMaxFileNameSize];
+ char new_file_name[FileWrapper::kMaxFileNameSize];
+
+ // get current name
+ trace_file_->FileName(old_file_name, FileWrapper::kMaxFileNameSize);
+ trace_file_->CloseFile();
+
+ file_count_text_++;
+
+ UpdateFileName(old_file_name, new_file_name, file_count_text_);
+
+ if (trace_file_->OpenFile(new_file_name, false, false, true) == -1) {
+ return;
+ }
}
- if (trace_file_.Open()) {
- if (row_count_text_ > WEBRTC_TRACE_MAX_FILE_SIZE) {
- // wrap file
- row_count_text_ = 0;
- trace_file_.Flush();
-
- if (file_count_text_ == 0) {
- trace_file_.Rewind();
- } else {
- char old_file_name[FileWrapper::kMaxFileNameSize];
- char new_file_name[FileWrapper::kMaxFileNameSize];
-
- // get current name
- trace_file_.FileName(old_file_name,
- FileWrapper::kMaxFileNameSize);
- trace_file_.CloseFile();
-
- file_count_text_++;
-
- UpdateFileName(old_file_name, new_file_name, file_count_text_);
-
- if (trace_file_.OpenFile(new_file_name, false, false,
- true) == -1) {
- return;
- }
- }
- }
- if (row_count_text_ == 0) {
- char message[WEBRTC_TRACE_MAX_MESSAGE_SIZE + 1];
- int32_t length = AddDateTimeInfo(message);
- if (length != -1) {
- message[length] = 0;
- message[length - 1] = '\n';
- trace_file_.Write(message, length);
- row_count_text_++;
- }
- }
- uint16_t length = length_[local_queue_active][idx];
- message_queue_[local_queue_active][idx][length] = 0;
- message_queue_[local_queue_active][idx][length - 1] = '\n';
- trace_file_.Write(message_queue_[local_queue_active][idx], length);
+ }
+ if (row_count_text_ == 0) {
+ char message[WEBRTC_TRACE_MAX_MESSAGE_SIZE + 1];
+ int32_t length = AddDateTimeInfo(message);
+ if (length != -1) {
+ message[length] = 0;
+ message[length - 1] = '\n';
+ trace_file_->Write(message, length);
row_count_text_++;
}
}
+ trace_file_->Write(msg, length);
+ row_count_text_++;
}
-void TraceImpl::AddImpl(const TraceLevel level, const TraceModule module,
+void TraceImpl::AddImpl(const TraceLevel level,
+ const TraceModule module,
const int32_t id,
const char msg[WEBRTC_TRACE_MAX_MESSAGE_SIZE]) {
- if (TraceCheck(level)) {
- char trace_message[WEBRTC_TRACE_MAX_MESSAGE_SIZE];
- char* message_ptr = trace_message;
+ if (!TraceCheck(level))
+ return;
- int32_t len = 0;
- int32_t ack_len = 0;
+ char trace_message[WEBRTC_TRACE_MAX_MESSAGE_SIZE];
+ char* message_ptr = &trace_message[0];
+ int32_t len = AddLevel(message_ptr, level);
+ if (len == -1)
+ return;
- len = AddLevel(message_ptr, level);
- if (len == -1) {
- return;
- }
- message_ptr += len;
- ack_len += len;
+ message_ptr += len;
+ int32_t ack_len = len;
- len = AddTime(message_ptr, level);
- if (len == -1) {
- return;
- }
- message_ptr += len;
- ack_len += len;
+ len = AddTime(message_ptr, level);
+ if (len == -1)
+ return;
- len = AddModuleAndId(message_ptr, module, id);
- if (len == -1) {
- return;
- }
- message_ptr += len;
- ack_len += len;
+ message_ptr += len;
+ ack_len += len;
- len = AddThreadId(message_ptr);
- if (len < 0) {
- return;
- }
- message_ptr += len;
- ack_len += len;
+ len = AddModuleAndId(message_ptr, module, id);
+ if (len == -1)
+ return;
- len = AddMessage(message_ptr, msg, (uint16_t)ack_len);
- if (len == -1) {
- return;
- }
- ack_len += len;
- AddMessageToList(trace_message, (uint16_t)ack_len, level);
+ message_ptr += len;
+ ack_len += len;
- // Make sure that messages are written as soon as possible.
- event_.Set();
- }
+ len = AddThreadId(message_ptr);
+ if (len < 0)
+ return;
+
+ message_ptr += len;
+ ack_len += len;
+
+ len = AddMessage(message_ptr, msg, static_cast<uint16_t>(ack_len));
+ if (len == -1)
+ return;
+
+ ack_len += len;
+ AddMessageToList(trace_message, static_cast<uint16_t>(ack_len), level);
}
bool TraceImpl::TraceCheck(const TraceLevel level) const {
diff --git a/webrtc/system_wrappers/source/trace_impl.h b/webrtc/system_wrappers/source/trace_impl.h
index 5fa6ce3..62223da 100644
--- a/webrtc/system_wrappers/source/trace_impl.h
+++ b/webrtc/system_wrappers/source/trace_impl.h
@@ -11,7 +11,8 @@
#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_TRACE_IMPL_H_
#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_TRACE_IMPL_H_
-#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/scoped_ptr.h"
#include "webrtc/system_wrappers/interface/event_wrapper.h"
#include "webrtc/system_wrappers/interface/file_wrapper.h"
#include "webrtc/system_wrappers/interface/static_instance.h"
@@ -20,16 +21,6 @@
namespace webrtc {
-// TODO(pwestin) WEBRTC_TRACE_MAX_QUEUE needs to be tweaked
-// TODO(hellner) the buffer should be close to how much the system can write to
-// file. Increasing the buffer will not solve anything. Sooner or
-// later the buffer is going to fill up anyways.
-#if defined(WEBRTC_IOS)
-#define WEBRTC_TRACE_MAX_QUEUE 2000
-#else
-#define WEBRTC_TRACE_MAX_QUEUE 8000
-#endif
-#define WEBRTC_TRACE_NUM_ARRAY 2
#define WEBRTC_TRACE_MAX_MESSAGE_SIZE 1024
// Total buffer size is WEBRTC_TRACE_NUM_ARRAY (number of buffer partitions) *
// WEBRTC_TRACE_MAX_QUEUE (number of lines per buffer partition) *
@@ -56,8 +47,6 @@
void AddImpl(const TraceLevel level, const TraceModule module,
const int32_t id, const char* msg);
- bool StopThread();
-
bool TraceCheck(const TraceLevel level) const;
protected:
@@ -74,9 +63,6 @@
virtual int32_t AddDateTimeInfo(char* trace_message) const = 0;
- static bool Run(void* obj);
- bool Process();
-
private:
friend class Trace;
@@ -104,24 +90,15 @@
char file_name_with_counter_utf8[FileWrapper::kMaxFileNameSize],
const uint32_t new_count) const;
- void WriteToFile();
+ void WriteToFile(const char* msg, uint16_t length)
+ EXCLUSIVE_LOCKS_REQUIRED(crit_);
- CriticalSectionWrapper* critsect_interface_;
- TraceCallback* callback_;
- uint32_t row_count_text_;
- uint32_t file_count_text_;
+ rtc::CriticalSection crit_;
+ TraceCallback* callback_ GUARDED_BY(crit_);
+ uint32_t row_count_text_ GUARDED_BY(crit_);
+ uint32_t file_count_text_ GUARDED_BY(crit_);
- FileWrapper& trace_file_;
- ThreadWrapper& thread_;
- EventWrapper& event_;
-
- // critsect_array_ protects active_queue_.
- CriticalSectionWrapper* critsect_array_;
- uint16_t next_free_idx_[WEBRTC_TRACE_NUM_ARRAY];
- TraceLevel level_[WEBRTC_TRACE_NUM_ARRAY][WEBRTC_TRACE_MAX_QUEUE];
- uint16_t length_[WEBRTC_TRACE_NUM_ARRAY][WEBRTC_TRACE_MAX_QUEUE];
- char* message_queue_[WEBRTC_TRACE_NUM_ARRAY][WEBRTC_TRACE_MAX_QUEUE];
- uint8_t active_queue_;
+ const rtc::scoped_ptr<FileWrapper> trace_file_ GUARDED_BY(crit_);
};
} // namespace webrtc
diff --git a/webrtc/system_wrappers/source/trace_posix.cc b/webrtc/system_wrappers/source/trace_posix.cc
index 1e0e1c8..cb702d8 100644
--- a/webrtc/system_wrappers/source/trace_posix.cc
+++ b/webrtc/system_wrappers/source/trace_posix.cc
@@ -28,7 +28,6 @@
TracePosix::~TracePosix() {
delete &crit_sect_;
- StopThread();
}
int32_t TracePosix::AddTime(char* trace_message, const TraceLevel level) const {
diff --git a/webrtc/system_wrappers/source/trace_win.cc b/webrtc/system_wrappers/source/trace_win.cc
index 3752659..4caedfc 100644
--- a/webrtc/system_wrappers/source/trace_win.cc
+++ b/webrtc/system_wrappers/source/trace_win.cc
@@ -22,7 +22,6 @@
}
TraceWindows::~TraceWindows() {
- StopThread();
}
int32_t TraceWindows::AddTime(char* trace_message,
diff --git a/webrtc/system_wrappers/system_wrappers.gyp b/webrtc/system_wrappers/system_wrappers.gyp
index 0ec3a32..f61a805 100644
--- a/webrtc/system_wrappers/system_wrappers.gyp
+++ b/webrtc/system_wrappers/system_wrappers.gyp
@@ -115,11 +115,6 @@
}, {
'sources!': [ 'source/data_log.cc', ],
},],
- ['rtc_use_direct_trace==1', {
- 'defines': [
- 'WEBRTC_DIRECT_TRACE',
- ],
- }],
['OS=="android"', {
'defines': [
'WEBRTC_THREAD_RR',
