Retire SampleOrderingSpec::PRESERVE_ORDER as it's not used

Since SampleOrderingSpec::MAY_SHUFFLE is the only one used in production
code, there is no need to specify the SampleOrderingSpec parameter hence
SampleOrderingSpec enum is also retired.

Bug: 851163
Change-Id: Ie9bf7c39d036db55b1533c4fe3b3f3293a2a8dbf
Reviewed-on: https://chromium-review.googlesource.com/1171584
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582379}

chrome/common/thread_profiler.cc

@@ -190,8 +190,7 @@ ThreadProfiler::ThreadProfiler(
     : owning_thread_task_runner_(owning_thread_task_runner),
       periodic_profile_params_(GetProcess(),
                                thread,
-                               CallStackProfileParams::PERIODIC_COLLECTION,
-                               CallStackProfileParams::MAY_SHUFFLE),
+                               CallStackProfileParams::PERIODIC_COLLECTION),
       weak_factory_(this) {
   if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForCurrentProcess())
     return;
@@ -200,8 +199,7 @@ ThreadProfiler::ThreadProfiler(
       BindRepeating(&ThreadProfiler::ReceiveStartupProfile,
                     GetReceiverCallback()),
       CallStackProfileParams(GetProcess(), thread,
-                             CallStackProfileParams::PROCESS_STARTUP,
-                             CallStackProfileParams::MAY_SHUFFLE));
+                             CallStackProfileParams::PROCESS_STARTUP));
 
   startup_profiler_ = std::make_unique<StackSamplingProfiler>(
       base::PlatformThread::CurrentId(), kSamplingParams,

components/metrics/call_stack_profile_builder.cc

@@ -99,8 +99,7 @@ void CallStackProfileBuilder::OnProfileCompleted(
   sampled_profile.set_thread(ToExecutionContextThread(profile_params_.thread));
   sampled_profile.set_trigger_event(
       ToSampledProfileTriggerEvent(profile_params_.trigger));
-  CopyProfileToProto(profile_, profile_params_.ordering_spec,
-                     sampled_profile.mutable_call_stack_profile());
+  CopyProfileToProto(profile_, sampled_profile.mutable_call_stack_profile());
 
   // Run the associated callback, passing the protocol message which encodes the
   // collected profile.

components/metrics/call_stack_profile_builder_unittest.cc

@@ -24,8 +24,7 @@ namespace {
 constexpr CallStackProfileParams kProfileParams = {
     CallStackProfileParams::BROWSER_PROCESS,
     CallStackProfileParams::MAIN_THREAD,
-    CallStackProfileParams::PROCESS_STARTUP,
-    CallStackProfileParams::MAY_SHUFFLE};
+    CallStackProfileParams::PROCESS_STARTUP};
 
 // Called on the profiler thread when complete, to collect profile.
 void SaveProfile(SampledProfile* proto, SampledProfile pending_proto) {

components/metrics/call_stack_profile_params.h

@@ -47,16 +47,6 @@ struct CallStackProfileParams {
     TRIGGER_LAST = PERIODIC_COLLECTION
   };
 
-  // Allows the caller to specify whether sample ordering is
-  // important. MAY_SHUFFLE should always be used to enable better compression,
-  // unless the use case needs order to be preserved for a specific reason.
-  enum SampleOrderingSpec {
-    // The provider may shuffle the sample order to improve compression.
-    MAY_SHUFFLE,
-    // The provider will not change the sample order.
-    PRESERVE_ORDER
-  };
-
   // The default constructor is required for mojo and should not be used
   // otherwise. A valid trigger should always be specified.
   constexpr CallStackProfileParams()
@@ -64,15 +54,7 @@ struct CallStackProfileParams {
   constexpr CallStackProfileParams(Process process,
                                    Thread thread,
                                    Trigger trigger)
-      : CallStackProfileParams(process, thread, trigger, MAY_SHUFFLE) {}
-  constexpr CallStackProfileParams(Process process,
-                                   Thread thread,
-                                   Trigger trigger,
-                                   SampleOrderingSpec ordering_spec)
-      : process(process),
-        thread(thread),
-        trigger(trigger),
-        ordering_spec(ordering_spec) {}
+      : process(process), thread(thread), trigger(trigger) {}
 
   // The collection process.
   Process process;
@@ -82,9 +64,6 @@ struct CallStackProfileParams {
 
   // The triggering event.
   Trigger trigger;
-
-  // Whether to preserve sample ordering.
-  SampleOrderingSpec ordering_spec;
 };
 
 }  // namespace metrics

components/metrics/call_stack_profile_proto_encoder.cc

@@ -84,30 +84,21 @@ void CopyAnnotationsToProto(uint32_t new_milestones,
   }
 }
 
+// The sample order in |profile| is not preserved in |proto_profile|.
 void CopyProfileToProto(
     const base::StackSamplingProfiler::CallStackProfile& profile,
-    CallStackProfileParams::SampleOrderingSpec ordering_spec,
     CallStackProfile* proto_profile) {
   if (profile.samples.empty())
     return;
 
-  const bool preserve_order =
-      ordering_spec == CallStackProfileParams::PRESERVE_ORDER;
-
   std::map<base::StackSamplingProfiler::Sample, int> sample_index;
   uint32_t milestones = 0;
   for (auto it = profile.samples.begin(); it != profile.samples.end(); ++it) {
     int existing_sample_index = -1;
-    if (preserve_order) {
-      // Collapse sample with the previous one if they match. Samples match
-      // if the frame and all annotations are the same.
-      if (proto_profile->sample_size() > 0 && *it == *(it - 1))
-        existing_sample_index = proto_profile->sample_size() - 1;
-    } else {
-      auto location = sample_index.find(*it);
-      if (location != sample_index.end())
-        existing_sample_index = location->second;
-    }
+
+    auto location = sample_index.find(*it);
+    if (location != sample_index.end())
+      existing_sample_index = location->second;
 
     if (existing_sample_index != -1) {
       CallStackProfile::Sample* sample_proto =
@@ -122,10 +113,8 @@ void CopyProfileToProto(
     CopyAnnotationsToProto(it->process_milestones & ~milestones, sample_proto);
     milestones = it->process_milestones;
 
-    if (!preserve_order) {
-      sample_index.insert(std::make_pair(
-          *it, static_cast<int>(proto_profile->sample_size()) - 1));
-    }
+    sample_index.insert(std::make_pair(
+        *it, static_cast<int>(proto_profile->sample_size()) - 1));
   }
 
   for (const auto& module : profile.modules) {

components/metrics/call_stack_profile_proto_encoder.h

@@ -37,7 +37,6 @@ void CopyAnnotationsToProto(uint32_t new_milestones,
 // Transcode |profile| into |proto_profile|.
 void CopyProfileToProto(
     const base::StackSamplingProfiler::CallStackProfile& profile,
-    CallStackProfileParams::SampleOrderingSpec ordering_spec,
     CallStackProfile* proto_profile);
 
 // Translates CallStackProfileParams's process to the corresponding

components/metrics/call_stack_profile_proto_encoder_unittest.cc

@@ -221,80 +221,7 @@ TEST(CallStackProfileProtoEncoderTest, RepeatedStacksUnordered) {
   };
 
   SampledProfile proto;
-  CopyProfileToProto(profile, CallStackProfileParams::MAY_SHUFFLE,
-                     proto.mutable_call_stack_profile());
-
-  VerifyProfileProto(expected_proto_profile, proto);
-}
-
-// Checks that only contiguous duplicate samples are collapsed with
-// preserve_sample_ordering = true.
-TEST(CallStackProfileProtoEncoderTest, RepeatedStacksOrdered) {
-  const uintptr_t module_base_address = 0x1000;
-  const char* module_name = "ABCD";
-
-#if defined(OS_WIN)
-  uint64_t module_md5 = 0x46C3E4166659AC02ULL;
-  base::FilePath module_path(L"c:\\some\\path\\to\\chrome.exe");
-#else
-  uint64_t module_md5 = 0x554838A8451AC36CULL;
-  base::FilePath module_path("/some/path/to/chrome");
-#endif
-
-  Profile profile =
-      ProfileFactory(100, 10)
-          .DefineModule(module_name, module_path, module_base_address)
-
-          .AddMilestone(0)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x10)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x20)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x10)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x10)
-
-          .AddMilestone(1)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x10)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x20)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x10)
-          .NewSample()
-          .AddFrame(0, module_base_address + 0x10)
-
-          .Build();
-
-  const ExpectedProtoModule expected_proto_modules[] = {
-      {module_name, module_md5, module_base_address},
-  };
-
-  const ExpectedProtoEntry expected_proto_entries[] = {
-      {0, 0x10}, {0, 0x20},
-  };
-  const ExpectedProtoSample expected_proto_samples[] = {
-      {1, &expected_proto_entries[0], 1, 1},
-      {0, &expected_proto_entries[1], 1, 1},
-      {0, &expected_proto_entries[0], 1, 2},
-      {2, &expected_proto_entries[0], 1, 1},
-      {0, &expected_proto_entries[1], 1, 1},
-      {0, &expected_proto_entries[0], 1, 2},
-  };
-
-  const ExpectedProtoProfile expected_proto_profile = {
-      100,
-      10,
-      expected_proto_modules,
-      base::size(expected_proto_modules),
-      expected_proto_samples,
-      base::size(expected_proto_samples),
-  };
-
-  SampledProfile proto;
-  CopyProfileToProto(profile, CallStackProfileParams::PRESERVE_ORDER,
-                     proto.mutable_call_stack_profile());
+  CopyProfileToProto(profile, proto.mutable_call_stack_profile());
 
   VerifyProfileProto(expected_proto_profile, proto);
 }
@@ -323,8 +250,7 @@ TEST(CallStackProfileProtoEncoderTest, UnknownModule) {
   };
 
   SampledProfile proto;
-  CopyProfileToProto(profile, CallStackProfileParams::MAY_SHUFFLE,
-                     proto.mutable_call_stack_profile());
+  CopyProfileToProto(profile, proto.mutable_call_stack_profile());
 
   VerifyProfileProto(expected_proto_profile, proto);
 }