Make interning interval configurable

While we figure out better solutions for interning, to make traces
useful we can decrease interning interval. Make this configurable by
experiments.

Change-Id: I9ad99ab3184362f81d9b7ba5bc217e9c8b1914bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1716601
Commit-Queue: ssid <ssid@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680611}

content/browser/tracing/background_tracing_active_scenario.cc

@@ -71,7 +71,8 @@ class PerfettoTracingSession
       public mojo::DataPipeDrainer::Client {
  public:
   PerfettoTracingSession(BackgroundTracingActiveScenario* parent_scenario,
-                         const TraceConfig& chrome_config)
+                         const TraceConfig& chrome_config,
+                         int interning_reset_interval_ms)
       : parent_scenario_(parent_scenario),
         raw_data_(std::make_unique<std::string>()) {
 #if !defined(OS_ANDROID)
@@ -89,6 +90,8 @@ class PerfettoTracingSession
 
     perfetto::TraceConfig perfetto_config = tracing::GetDefaultPerfettoConfig(
         chrome_config, /*privacy_filtering_enabled=*/true);
+    perfetto_config.mutable_incremental_state_config()->set_clear_period_ms(
+        interning_reset_interval_ms);
 
     tracing::mojom::TracingSessionClientPtr tracing_session_client;
     binding_.Bind(mojo::MakeRequest(&tracing_session_client));
@@ -264,11 +267,9 @@ class LegacyTracingSession
 
 BackgroundTracingActiveScenario::BackgroundTracingActiveScenario(
     std::unique_ptr<BackgroundTracingConfigImpl> config,
-    bool requires_anonymized_data,
     BackgroundTracingManager::ReceiveCallback receive_callback,
     base::OnceClosure on_aborted_callback)
     : config_(std::move(config)),
-      requires_anonymized_data_(requires_anonymized_data),
       receive_callback_(std::move(receive_callback)),
       on_aborted_callback_(std::move(on_aborted_callback)) {
   DCHECK(config_ && !config_->rules().empty());
@@ -333,8 +334,7 @@ void BackgroundTracingActiveScenario::StartTracingIfConfigNeedsIt() {
 }
 
 bool BackgroundTracingActiveScenario::StartTracing() {
-  TraceConfig chrome_config =
-      config_->GetTraceConfig(requires_anonymized_data_);
+  TraceConfig chrome_config = config_->GetTraceConfig();
 
   // If the tracing controller is tracing, i.e. DevTools or about://tracing,
   // we don't start background tracing to not interfere with the user activity.
@@ -354,8 +354,8 @@ bool BackgroundTracingActiveScenario::StartTracing() {
 
   DCHECK(!tracing_session_);
   if (base::FeatureList::IsEnabled(features::kBackgroundTracingProtoOutput)) {
-    tracing_session_ =
-        std::make_unique<PerfettoTracingSession>(this, chrome_config);
+    tracing_session_ = std::make_unique<PerfettoTracingSession>(
+        this, chrome_config, config_->interning_reset_interval_ms());
   } else {
     tracing_session_ =
         std::make_unique<LegacyTracingSession>(this, chrome_config);

content/browser/tracing/background_tracing_active_scenario.h

@@ -31,7 +31,6 @@ class BackgroundTracingActiveScenario {
 
   BackgroundTracingActiveScenario(
       std::unique_ptr<BackgroundTracingConfigImpl> config,
-      bool requires_anonymized_data,
       BackgroundTracingManager::ReceiveCallback receive_callback,
       base::OnceClosure on_aborted_callback);
   virtual ~BackgroundTracingActiveScenario();
@@ -39,12 +38,11 @@ class BackgroundTracingActiveScenario {
   void StartTracingIfConfigNeedsIt();
   void AbortScenario();
 
-  const BackgroundTracingConfigImpl* GetConfig() const;
+  CONTENT_EXPORT const BackgroundTracingConfigImpl* GetConfig() const;
   void GenerateMetadataDict(base::DictionaryValue* metadata_dict);
   void GenerateMetadataProto(
       perfetto::protos::pbzero::ChromeMetadataPacket* metadata);
   State state() const { return scenario_state_; }
-  bool requires_anonymized_data() const { return requires_anonymized_data_; }
   base::WeakPtr<BackgroundTracingActiveScenario> GetWeakPtr();
 
   void TriggerNamedEvent(
@@ -86,7 +84,6 @@ class BackgroundTracingActiveScenario {
   std::unique_ptr<BackgroundTracingConfigImpl> config_;
   // Owned by |config_|.
   const BackgroundTracingRule* last_triggered_rule_ = nullptr;
-  const bool requires_anonymized_data_ = false;
   State scenario_state_ = State::kIdle;
   base::RepeatingClosure rule_triggered_callback_for_testing_;
   BackgroundTracingManager::ReceiveCallback receive_callback_;

content/browser/tracing/background_tracing_config_impl.cc

@@ -56,6 +56,7 @@ const char kConfigMobileNetworkBuferSizeKb[] = "mobile_network_buffer_size_kb";
 const char kConfigMaxBufferSizeKb[] = "max_buffer_size_kb";
 const char kConfigUploadLimitKb[] = "upload_limit_kb";
 const char kConfigUploadLimitNetworkKb[] = "upload_limit_network_kb";
+const char kConfigInterningResetIntervalMs[] = "interning_reset_interval_ms";
 
 }  // namespace
 
@@ -234,8 +235,7 @@ void BackgroundTracingConfigImpl::AddReactiveRule(
   }
 }
 
-TraceConfig BackgroundTracingConfigImpl::GetTraceConfig(
-    bool requires_anonymized_data) {
+TraceConfig BackgroundTracingConfigImpl::GetTraceConfig() const {
   base::trace_event::TraceRecordMode record_mode =
       (tracing_mode() == BackgroundTracingConfigImpl::REACTIVE)
           ? base::trace_event::RECORD_UNTIL_FULL
@@ -251,7 +251,7 @@ TraceConfig BackgroundTracingConfigImpl::GetTraceConfig(
     chrome_config.SetProcessFilterConfig(process_config);
   }
 
-  if (requires_anonymized_data) {
+  if (requires_anonymized_data_) {
     chrome_config.EnableArgumentFilter();
   }
 
@@ -510,6 +510,9 @@ void BackgroundTracingConfigImpl::SetBufferSizeLimits(
   if (dict->GetInteger(kConfigUploadLimitNetworkKb, &value)) {
     upload_limit_network_kb_ = value;
   }
+  if (dict->GetInteger(kConfigInterningResetIntervalMs, &value)) {
+    interning_reset_interval_ms_ = value;
+  }
 }
 
 int BackgroundTracingConfigImpl::GetMaximumTraceBufferSizeKb() const {

content/browser/tracing/background_tracing_config_impl.h

@@ -63,9 +63,17 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
       const base::DictionaryValue* dict,
       BackgroundTracingConfigImpl::CategoryPreset category_preset);
 
-  base::trace_event::TraceConfig GetTraceConfig(bool requires_anonymized_data);
+  base::trace_event::TraceConfig GetTraceConfig() const;
 
   size_t GetTraceUploadLimitKb() const;
+  int interning_reset_interval_ms() const {
+    return interning_reset_interval_ms_;
+  }
+
+  void set_requires_anonymized_data(bool value) {
+    requires_anonymized_data_ = value;
+  }
+  bool requires_anonymized_data() const { return requires_anonymized_data_; }
 
   static std::unique_ptr<BackgroundTracingConfigImpl> PreemptiveFromDict(
       const base::DictionaryValue* dict);
@@ -107,6 +115,7 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
   std::string scenario_name_;
   std::string custom_categories_;
 
+  bool requires_anonymized_data_ = false;
   bool trace_browser_process_only_ = false;
 
   // The default memory overhead of running background tracing for various
@@ -124,6 +133,7 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
   // compression ratio grows up to 8x if the buffer size is around 100MB.
   int upload_limit_network_kb_ = 1024;
   int upload_limit_kb_ = kUploadLimitKb;
+  int interning_reset_interval_ms_ = 5000;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundTracingConfigImpl);
 };

content/browser/tracing/background_tracing_config_unittest.cc

@@ -690,12 +690,12 @@ TEST_F(BackgroundTracingConfigTest, BufferLimitConfig) {
 
   notifier.set_type(net::NetworkChangeNotifier::CONNECTION_2G);
 #if defined(OS_ANDROID)
-  EXPECT_EQ(300u, config->GetTraceConfig(true).GetTraceBufferSizeInKb());
+  EXPECT_EQ(300u, config->GetTraceConfig().GetTraceBufferSizeInKb());
   EXPECT_EQ(600u, config->GetTraceUploadLimitKb());
 #endif
 
   notifier.set_type(net::NetworkChangeNotifier::CONNECTION_WIFI);
-  EXPECT_LE(800u, config->GetTraceConfig(true).GetTraceBufferSizeInKb());
+  EXPECT_LE(800u, config->GetTraceConfig().GetTraceBufferSizeInKb());
   EXPECT_EQ(500u, config->GetTraceUploadLimitKb());
 }
 

content/browser/tracing/background_tracing_manager_browsertest.cc

@@ -1407,6 +1407,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest, RunStartupTracing) {
 
   EXPECT_TRUE(BackgroundTracingManagerImpl::GetInstance()
                   ->GetActiveScenarioForTesting()
+                  ->GetConfig()
                   ->requires_anonymized_data());
   EXPECT_TRUE(base::trace_event::TraceLog::GetInstance()
                   ->GetCurrentTraceConfig()

content/browser/tracing/background_tracing_manager_impl.cc

@@ -145,6 +145,7 @@ bool BackgroundTracingManagerImpl::SetActiveScenario(
   }
 
   bool requires_anonymized_data = (data_filtering == ANONYMIZE_DATA);
+  config_impl->set_requires_anonymized_data(requires_anonymized_data);
 
   // If the profile hasn't loaded or been created yet, this is a startup
   // scenario and we have to wait until initialization is finished to validate
@@ -169,8 +170,7 @@ bool BackgroundTracingManagerImpl::SetActiveScenario(
   }
 
   active_scenario_ = std::make_unique<BackgroundTracingActiveScenario>(
-      std::move(config_impl), requires_anonymized_data,
-      std::move(receive_callback),
+      std::move(config_impl), std::move(receive_callback),
       base::BindOnce(&BackgroundTracingManagerImpl::OnScenarioAborted,
                      base::Unretained(this)));
 
@@ -307,7 +307,7 @@ void BackgroundTracingManagerImpl::ValidateStartupScenario() {
 
   if (!delegate_->IsAllowedToBeginBackgroundScenario(
           *active_scenario_->GetConfig(),
-          active_scenario_->requires_anonymized_data())) {
+          active_scenario_->GetConfig()->requires_anonymized_data())) {
     AbortScenario();
   }
 }
@@ -398,10 +398,11 @@ void BackgroundTracingManagerImpl::WhenIdle(
 }
 
 bool BackgroundTracingManagerImpl::IsAllowedFinalization() const {
-  return !delegate_ || (active_scenario_ &&
-                        delegate_->IsAllowedToEndBackgroundScenario(
-                            *active_scenario_->GetConfig(),
-                            active_scenario_->requires_anonymized_data()));
+  return !delegate_ ||
+         (active_scenario_ &&
+          delegate_->IsAllowedToEndBackgroundScenario(
+              *active_scenario_->GetConfig(),
+              active_scenario_->GetConfig()->requires_anonymized_data()));
 }
 
 std::unique_ptr<base::DictionaryValue>