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 3580d9c076..dfa59ed10e 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 +#include #include #include "logging/rtc_event_log/events/rtc_event.h" @@ -21,6 +23,10 @@ class RtcEventLogEncoder { virtual ~RtcEventLogEncoder() = default; virtual std::string Encode(const RtcEvent& event) = 0; + + virtual std::string EncodeBatch( + std::deque>::const_iterator begin, + std::deque>::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 dd4abd9bb4..0f3f3be201 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 @@ rtclog::VideoReceiveConfig_RtcpMode ConvertRtcpMode(RtcpMode rtcp_mode) { } } // namespace +std::string RtcEventLogEncoderLegacy::EncodeBatch( + std::deque>::const_iterator begin, + std::deque>::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 f4d3355a97..9f795a6501 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 #include #include @@ -51,6 +52,10 @@ class RtcEventLogEncoderLegacy final : public RtcEventLogEncoder { std::string Encode(const RtcEvent& event) override; + std::string EncodeBatch( + std::deque>::const_iterator begin, + std::deque>::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 9bbccafdc0..c6323c0d88 100644 --- a/logging/rtc_event_log/rtc_event_log.cc +++ b/logging/rtc_event_log/rtc_event_log.cc @@ -95,22 +95,14 @@ class RtcEventLogImpl final : public RtcEventLog { void Log(std::unique_ptr 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 event) RTC_RUN_ON(&task_queue_); - void LogEventsFromMemoryToOutput() RTC_RUN_ON(&task_queue_); - void LogToOutput(std::unique_ptr 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 @@ bool RtcEventLogImpl::StartLogging(std::unique_ptr output, RTC_DCHECK(output->IsActive()); event_output_ = std::move(output); num_config_events_written_ = 0; - LogToOutput(rtc::MakeUnique(start_event)); + WriteToOutput(event_encoder_->Encode(start_event)); LogEventsFromMemoryToOutput(); }; @@ -279,26 +271,6 @@ void RtcEventLogImpl::ScheduleOutput() { } } -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 event) { std::deque>& container = event->IsConfigEvent() ? config_history_ : history_; @@ -316,65 +288,46 @@ void RtcEventLogImpl::LogEventsFromMemoryToOutput() { 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 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::StopOutput() { void RtcEventLogImpl::StopLoggingInternal() { if (event_output_) { RTC_DCHECK(event_output_->IsActive()); - event_output_->Write( - event_encoder_->Encode(*rtc::MakeUnique())); + RtcEventLoggingStopped stop_event; + event_output_->Write(event_encoder_->Encode(stop_event)); } StopOutput(); }