Add method to encode a batch of RtcEvents.
Bug: webrtc:8111
Change-Id: Ieb3ff6b817a6bc068b358d49e6d6df07e3ec8d8b
Reviewed-on: https://webrtc-review.googlesource.com/32720
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21286}diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder.h b/logging/rtc_event_log/encoder/rtc_event_log_encoder.h
index 3580d9c..dfa59ed 100644
--- a/logging/rtc_event_log/encoder/rtc_event_log_encoder.h
+++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder.h
@@ -11,6 +11,8 @@
#ifndef LOGGING_RTC_EVENT_LOG_ENCODER_RTC_EVENT_LOG_ENCODER_H_
#define LOGGING_RTC_EVENT_LOG_ENCODER_RTC_EVENT_LOG_ENCODER_H_
+#include <deque>
+#include <memory>
#include <string>
#include "logging/rtc_event_log/events/rtc_event.h"
@@ -21,6 +23,10 @@
virtual ~RtcEventLogEncoder() = default;
virtual std::string Encode(const RtcEvent& event) = 0;
+
+ virtual std::string EncodeBatch(
+ std::deque<std::unique_ptr<RtcEvent>>::const_iterator begin,
+ std::deque<std::unique_ptr<RtcEvent>>::const_iterator end) = 0;
};
} // namespace webrtc
diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc
index dd4abd9..0f3f3be 100644
--- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc
+++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc
@@ -107,6 +107,18 @@
}
} // namespace
+std::string RtcEventLogEncoderLegacy::EncodeBatch(
+ std::deque<std::unique_ptr<RtcEvent>>::const_iterator begin,
+ std::deque<std::unique_ptr<RtcEvent>>::const_iterator end) {
+ std::string encoded_output;
+ for (auto it = begin; it != end; ++it) {
+ // TODO(terelius): Can we avoid the slight inefficiency of reallocating the
+ // string?
+ encoded_output += Encode(**it);
+ }
+ return encoded_output;
+}
+
std::string RtcEventLogEncoderLegacy::Encode(const RtcEvent& event) {
switch (event.GetType()) {
case RtcEvent::Type::AudioNetworkAdaptation: {
diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h
index f4d3355..9f795a6 100644
--- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h
+++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h
@@ -11,6 +11,7 @@
#ifndef LOGGING_RTC_EVENT_LOG_ENCODER_RTC_EVENT_LOG_ENCODER_LEGACY_H_
#define LOGGING_RTC_EVENT_LOG_ENCODER_RTC_EVENT_LOG_ENCODER_LEGACY_H_
+#include <deque>
#include <memory>
#include <string>
@@ -51,6 +52,10 @@
std::string Encode(const RtcEvent& event) override;
+ std::string EncodeBatch(
+ std::deque<std::unique_ptr<RtcEvent>>::const_iterator begin,
+ std::deque<std::unique_ptr<RtcEvent>>::const_iterator end) override;
+
private:
// Encoding entry-point for the various RtcEvent subclasses.
std::string EncodeAlrState(const RtcEventAlrState& event);
diff --git a/logging/rtc_event_log/rtc_event_log.cc b/logging/rtc_event_log/rtc_event_log.cc
index 9bbccaf..c6323c0 100644
--- a/logging/rtc_event_log/rtc_event_log.cc
+++ b/logging/rtc_event_log/rtc_event_log.cc
@@ -95,22 +95,14 @@
void Log(std::unique_ptr<RtcEvent> event) override;
private:
- // Appends an event to the output protobuf string, returning true on success.
- // Fails and returns false in case the limit on output size prevents the
- // event from being added; in this case, the output string is left unchanged.
- // The event is encoded before being appended.
- // We could have avoided this, because the output repeats the check, but this
- // way, we minimize the number of lock acquisitions, task switches, etc.,
- // that might be associated with each call to RtcEventLogOutput::Write().
- bool AppendEventToString(const RtcEvent& event,
- std::string* output_string) RTC_WARN_UNUSED_RESULT;
-
void LogToMemory(std::unique_ptr<RtcEvent> event) RTC_RUN_ON(&task_queue_);
-
void LogEventsFromMemoryToOutput() RTC_RUN_ON(&task_queue_);
- void LogToOutput(std::unique_ptr<RtcEvent> event) RTC_RUN_ON(&task_queue_);
+
void StopOutput() RTC_RUN_ON(&task_queue_);
+ void WriteConfigsAndHistoryToOutput(const std::string& encoded_configs,
+ const std::string& encoded_history)
+ RTC_RUN_ON(&task_queue_);
void WriteToOutput(const std::string& output_string) RTC_RUN_ON(&task_queue_);
void StopLoggingInternal() RTC_RUN_ON(&task_queue_);
@@ -196,7 +188,7 @@
RTC_DCHECK(output->IsActive());
event_output_ = std::move(output);
num_config_events_written_ = 0;
- LogToOutput(rtc::MakeUnique<RtcEventLoggingStarted>(start_event));
+ WriteToOutput(event_encoder_->Encode(start_event));
LogEventsFromMemoryToOutput();
};
@@ -279,26 +271,6 @@
}
}
-bool RtcEventLogImpl::AppendEventToString(const RtcEvent& event,
- std::string* output_string) {
- RTC_DCHECK_RUN_ON(&task_queue_);
-
- std::string encoded_event = event_encoder_->Encode(event);
-
- bool appended;
- size_t potential_new_size =
- written_bytes_ + output_string->size() + encoded_event.length();
- if (potential_new_size <= max_size_bytes_) {
- // TODO(eladalon): This is inefficient; fix this in a separate CL.
- *output_string += encoded_event;
- appended = true;
- } else {
- appended = false;
- }
-
- return appended;
-}
-
void RtcEventLogImpl::LogToMemory(std::unique_ptr<RtcEvent> event) {
std::deque<std::unique_ptr<RtcEvent>>& container =
event->IsConfigEvent() ? config_history_ : history_;
@@ -316,65 +288,46 @@
RTC_DCHECK(event_output_ && event_output_->IsActive());
last_output_ms_ = rtc::TimeMillis();
- std::string output_string;
-
// Serialize all stream configurations that haven't already been written to
// this output. |num_config_events_written_| is used to track which configs we
// have already written. (Note that the config may have been written to
// previous outputs; configs are not discarded.)
- bool appended = true;
- while (num_config_events_written_ < config_history_.size()) {
- appended = AppendEventToString(*config_history_[num_config_events_written_],
- &output_string);
- if (!appended)
- break;
- ++num_config_events_written_;
+ std::string encoded_configs;
+ RTC_DCHECK_LE(num_config_events_written_, config_history_.size());
+ if (num_config_events_written_ < config_history_.size()) {
+ const auto begin = config_history_.begin() + num_config_events_written_;
+ const auto end = config_history_.end();
+ encoded_configs = event_encoder_->EncodeBatch(begin, end);
+ num_config_events_written_ = config_history_.size();
}
- // Serialize the events in the event queue.
- while (appended && !history_.empty()) {
- appended = AppendEventToString(*history_.front(), &output_string);
- if (appended) {
- // Known issue - if writing to the output fails, these events will have
- // been lost. If we try to open a new output, these events will be missing
- // from it.
- history_.pop_front();
- }
- }
+ // Serialize the events in the event queue. Note that the write may fail,
+ // for example if we are writing to a file and have reached the maximum limit.
+ // We don't get any feedback if this happens, so we still remove the events
+ // from the event log history. This is normally not a problem, but if another
+ // log is started immediately after the first one becomes full, then one
+ // cannot rely on the second log to contain everything that isn't in the first
+ // log; one batch of events might be missing.
+ std::string encoded_history =
+ event_encoder_->EncodeBatch(history_.begin(), history_.end());
+ history_.clear();
- WriteToOutput(output_string);
-
- if (!appended) {
- // Successful partial write to the output. Some events could not be written;
- // the output should be closed, to avoid gaps.
- StopOutput();
- }
+ WriteConfigsAndHistoryToOutput(encoded_configs, encoded_history);
}
-void RtcEventLogImpl::LogToOutput(std::unique_ptr<RtcEvent> event) {
- RTC_DCHECK(event_output_ && event_output_->IsActive());
-
- std::string output_string;
-
- bool appended = AppendEventToString(*event, &output_string);
-
- if (event->IsConfigEvent()) {
- // Config events need to be kept in memory too, so that they may be
- // rewritten into future outputs, too.
- config_history_.push_back(std::move(event));
+void RtcEventLogImpl::WriteConfigsAndHistoryToOutput(
+ const std::string& encoded_configs,
+ const std::string& encoded_history) {
+ // This function is used to merge the strings instead of calling the output
+ // object twice with small strings. The function also avoids copying any
+ // strings in the typical case where there are no config events.
+ if (encoded_configs.size() == 0) {
+ WriteToOutput(encoded_history); // Typical case.
+ } else if (encoded_history.size() == 0) {
+ WriteToOutput(encoded_configs); // Very unusual case.
+ } else {
+ WriteToOutput(encoded_configs + encoded_history);
}
-
- if (!appended) {
- if (!event->IsConfigEvent()) {
- // This event will not fit into the output; push it into |history_|
- // instead, so that it might be logged into the next output (if any).
- history_.push_back(std::move(event));
- }
- StopOutput();
- return;
- }
-
- WriteToOutput(output_string);
}
void RtcEventLogImpl::StopOutput() {
@@ -386,8 +339,8 @@
void RtcEventLogImpl::StopLoggingInternal() {
if (event_output_) {
RTC_DCHECK(event_output_->IsActive());
- event_output_->Write(
- event_encoder_->Encode(*rtc::MakeUnique<RtcEventLoggingStopped>()));
+ RtcEventLoggingStopped stop_event;
+ event_output_->Write(event_encoder_->Encode(stop_event));
}
StopOutput();
}
