Revert "Reland "[ssm] Add field trial for java name hashing on androi…

…d."" This reverts commit 96d68fc. Reason for revert: The new expectation breaks at https://ci.chromium.org/ui/p/chrome/builders/ci/android-arm-tests/21025/overview [ RUN ] ThreadProfilerPlatformConfigurationTest.GetEnableRates ../../chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc:90: Failure Expected equality of these values: (RelativePopulations{1, 99}) Which is: {1, 99} config()->GetEnableRates(version_info::Channel::CANARY) Which is: {80, 20} Stack trace: ../../chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc:92: Failure Expected equality of these values: (RelativePopulations{1, 99}) Which is: {1, 99} config()->GetEnableRates(version_info::Channel::DEV) Which is: {80, 20} Original change's description: > Reland "[ssm] Add field trial for java name hashing on android." > > This is a reland of commit 6a966a6 > > Original change's description: > > [ssm] Add field trial for java name hashing on android. > > > > Bug: 1475718 > > Change-Id: I6e8db8f2967743368488d03bdf983c06ae453484 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4864324 > > Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> > > Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com> > > Reviewed-by: Mike Wittman <wittman@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1207443} > > Bug: 1475718 > Change-Id: I0b7dc21d288217db7644b9d483e08b49532a23ad > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942295 > Auto-Submit: Thiabaud Engelbrecht <thiabaud@google.com> > Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com> > Reviewed-by: Mike Wittman <wittman@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1211132} Bug: 1475718 Change-Id: I0f882b64829cb166a1399dde05a7fa10e998cc1a No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4952176 Auto-Submit: Friedrich Horschig <fhorschig@chromium.org> Commit-Queue: Friedrich Horschig <fhorschig@chromium.org> Owners-Override: Friedrich Horschig <fhorschig@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#1211434}
chromium · Oct 18, 2023 · 6c45566 · 6c45566
1 parent 3c72ee3
commit 6c45566
Show file tree

Hide file tree

Showing 10 changed files with 11 additions and 95 deletions.
diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc
@@ -1080,12 +1080,7 @@ void ChromeMainDelegate::SetupTracing() {
   // sampler profiler because it can support java frames which is essential for
   // the main thread.
   base::RepeatingCallback tracing_factory =
-#if BUILDFLAG(IS_ANDROID)
-      base::BindRepeating(&CreateCoreUnwindersFactory,
-                          /*is_java_name_hashing_enabled=*/false);
-#else
       base::BindRepeating(&CreateCoreUnwindersFactory);
-#endif  // BUILDFLAG(IS_ANDROID)
   tracing::TracingSamplerProfiler::UnwinderType unwinder_type =
       tracing::TracingSamplerProfiler::UnwinderType::kCustomAndroid;
 #if BUILDFLAG(IS_ANDROID)

diff --git a/chrome/common/profiler/thread_profiler.cc b/chrome/common/profiler/thread_profiler.cc
@@ -253,12 +253,7 @@ ThreadProfiler::ThreadProfiler(
               process_, thread,
               CallStackProfileParams::Trigger::kProcessStartup),
           work_id_recorder_.get()),
-#if BUILDFLAG(IS_ANDROID)
-      CreateCoreUnwindersFactory(
-          ThreadProfilerConfiguration::Get()->IsJavaNameHashingEnabled()),
-#else
       CreateCoreUnwindersFactory(),
-#endif  // BUILDFLAG(IS_ANDROID)
       GetApplyPerSampleMetadataCallback(process_));
 
   startup_profiler_->Start();
@@ -327,12 +322,7 @@ void ThreadProfiler::StartPeriodicSamplingCollection() {
           base::BindOnce(&ThreadProfiler::OnPeriodicCollectionCompleted,
                          owning_thread_task_runner_,
                          weak_factory_.GetWeakPtr())),
-#if BUILDFLAG(IS_ANDROID)
-      CreateCoreUnwindersFactory(
-          ThreadProfilerConfiguration::Get()->IsJavaNameHashingEnabled()),
-#else
       CreateCoreUnwindersFactory(),
-#endif  // BUILDFLAG(IS_ANDROID)
       GetApplyPerSampleMetadataCallback(process_));
   if (aux_unwinder_factory_)
     periodic_profiler_->AddAuxUnwinder(aux_unwinder_factory_.Run());

diff --git a/chrome/common/profiler/thread_profiler_configuration.cc b/chrome/common/profiler/thread_profiler_configuration.cc
@@ -112,12 +112,6 @@ bool ThreadProfilerConfiguration::GetSyntheticFieldTrial(
       *group_name = "Control";
       break;
 
-#if BUILDFLAG(IS_ANDROID)
-    case kProfileEnabledWithJavaNameHashing:
-      *group_name = "EnabledWithJavaNameHashing";
-      break;
-#endif  // BUILDFLAG(IS_ANDROID)
-
     case kProfileEnabled:
       *group_name = "Enabled";
       break;
@@ -157,13 +151,6 @@ void ThreadProfilerConfiguration::AppendCommandLineSwitchForChildProcess(
   }
 }
 
-#if BUILDFLAG(IS_ANDROID)
-bool ThreadProfilerConfiguration::IsJavaNameHashingEnabled() const {
-  const auto& config = absl::get<BrowserProcessConfiguration>(configuration_);
-  return config.variation_group == kProfileEnabledWithJavaNameHashing;
-}
-#endif  // BUILDFLAG(IS_ANDROID)
-
 ThreadProfilerConfiguration::ThreadProfilerConfiguration()
     : platform_configuration_(ThreadProfilerPlatformConfiguration::Create(
           IsBrowserTestModeEnabled())),
@@ -176,12 +163,8 @@ bool ThreadProfilerConfiguration::EnableForVariationGroup(
     absl::optional<VariationGroup> variation_group) {
   // Enable if assigned to a variation group, and the group is one of the groups
   // that are to be enabled.
-  return variation_group.has_value() &&
-         (*variation_group == kProfileEnabled ||
-#if BUILDFLAG(IS_ANDROID)
-          *variation_group == kProfileEnabledWithJavaNameHashing ||
-#endif  // BUILDFLAG(IS_ANDROID)
-          *variation_group == kProfileControl);
+  return variation_group.has_value() && (*variation_group == kProfileEnabled ||
+                                         *variation_group == kProfileControl);
 }
 
 // static
@@ -244,25 +227,13 @@ ThreadProfilerConfiguration::GenerateBrowserProcessConfiguration(
   const absl::optional<metrics::CallStackProfileParams::Process>
       process_type_to_sample = platform_configuration.ChooseEnabledProcess();
 
-#if BUILDFLAG(IS_ANDROID)
-  CHECK_EQ(0, relative_populations.experiment % 3);
-  return {ChooseVariationGroup({
-              {kProfileEnabled, relative_populations.enabled},
-              {kProfileControl, relative_populations.experiment / 3},
-              {kProfileEnabledWithJavaNameHashing,
-               relative_populations.experiment / 3},
-              {kProfileDisabled, relative_populations.experiment / 3},
-          }),
-          process_type_to_sample};
-#else
   CHECK_EQ(0, relative_populations.experiment % 2);
   return {ChooseVariationGroup({
               {kProfileEnabled, relative_populations.enabled},
               {kProfileControl, relative_populations.experiment / 2},
               {kProfileDisabled, relative_populations.experiment / 2},
           }),
           process_type_to_sample};
-#endif
 }
 
 // static

diff --git a/chrome/common/profiler/thread_profiler_configuration.h b/chrome/common/profiler/thread_profiler_configuration.h
@@ -60,10 +60,6 @@ class ThreadProfilerConfiguration {
   void AppendCommandLineSwitchForChildProcess(
       base::CommandLine* command_line) const;
 
-#if BUILDFLAG(IS_ANDROID)
-  bool IsJavaNameHashingEnabled() const;
-#endif  // BUILDFLAG(IS_ANDROID)
-
  private:
   friend base::NoDestructor<ThreadProfilerConfiguration>;
 
@@ -81,13 +77,6 @@ class ThreadProfilerConfiguration {
     // kProfileDisabled group).
     kProfileControl,
 
-#if BUILDFLAG(IS_ANDROID)
-    // Enabled within the experiment (and paired with equal-sized
-    // kProfileDisabled and kProfileControl groups). Java names will be
-    // hashed within this group.
-    kProfileEnabledWithJavaNameHashing,
-#endif  // BUILDFLAG(IS_ANDROID)
-
     // Enabled outside of the experiment.
     kProfileEnabled,
   };

diff --git a/chrome/common/profiler/thread_profiler_platform_configuration.cc b/chrome/common/profiler/thread_profiler_platform_configuration.cc
@@ -71,14 +71,7 @@ DefaultPlatformConfiguration::GetEnableRates(
   CHECK(*release_channel == version_info::Channel::CANARY ||
         *release_channel == version_info::Channel::DEV);
 
-#if BUILDFLAG(IS_ANDROID)
-  // This is temporary, in order to run the Java Name Hashing field trial.
-  //
-  // TODO(crbug.com/1475718): Remove this once the field trial is done.
-  return RelativePopulations{1, 99};
-#else
   return RelativePopulations{80, 20};
-#endif  // BUILDFLAG(IS_ANDROID)
 }
 
 double DefaultPlatformConfiguration::GetChildProcessPerExecutionEnableFraction(

diff --git a/chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc b/chrome/common/profiler/thread_profiler_platform_configuration_unittest.cc
@@ -86,9 +86,9 @@ MAYBE_PLATFORM_CONFIG_TEST_F(ThreadProfilerPlatformConfigurationTest,
   using RelativePopulations =
       ThreadProfilerPlatformConfiguration::RelativePopulations;
 #if BUILDFLAG(IS_ANDROID)
-  EXPECT_EQ((RelativePopulations{1, 99}),
+  EXPECT_EQ((RelativePopulations{80, 20}),
             config()->GetEnableRates(version_info::Channel::CANARY));
-  EXPECT_EQ((RelativePopulations{1, 99}),
+  EXPECT_EQ((RelativePopulations{80, 20}),
             config()->GetEnableRates(version_info::Channel::DEV));
   // Note: death tests aren't supported on Android. Otherwise this test would
   // check that the other inputs result in CHECKs.

diff --git a/chrome/common/profiler/unwind_util.cc b/chrome/common/profiler/unwind_util.cc
@@ -122,8 +122,7 @@ class ChromeUnwinderCreator {
 
 #if ANDROID_UNWINDING_SUPPORTED
 std::vector<std::unique_ptr<base::Unwinder>> CreateCoreUnwinders(
-    stack_unwinder::Module* const stack_unwinder_module,
-    bool is_java_name_hashing_enabled) {
+    stack_unwinder::Module* const stack_unwinder_module) {
   CHECK_NE(getpid(), gettid());
 
   static base::NoDestructor<NativeUnwinderAndroidMapDelegateImpl> map_delegate(
@@ -138,7 +137,7 @@ std::vector<std::unique_ptr<base::Unwinder>> CreateCoreUnwinders(
   std::vector<std::unique_ptr<base::Unwinder>> unwinders;
   unwinders.push_back(stack_unwinder_module->CreateNativeUnwinder(
       map_delegate.get(), reinterpret_cast<uintptr_t>(&__executable_start),
-      is_java_name_hashing_enabled));
+      /*is_java_name_hashing_enabled=*/false));
   unwinders.push_back(chrome_unwinder_creator->Create());
   return unwinders;
 }
@@ -245,18 +244,12 @@ stack_unwinder::Module* GetOrLoadModule() {
 }
 #endif  // ANDROID_UNWINDING_SUPPORTED
 
-#if BUILDFLAG(IS_ANDROID)
-base::StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactory(
-    bool is_java_name_hashing_enabled) {
-#else
 base::StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactory() {
-#endif  // BUILDFLAG(IS_ANDROID)
   if (!AreUnwindPrerequisitesAvailable(chrome::GetChannel())) {
     return base::StackSamplingProfiler::UnwindersFactory();
   }
 #if ANDROID_UNWINDING_SUPPORTED
-  return base::BindOnce(CreateCoreUnwinders, GetOrLoadModule(),
-                        is_java_name_hashing_enabled);
+  return base::BindOnce(CreateCoreUnwinders, GetOrLoadModule());
 #else   // ANDROID_UNWINDING_SUPPORTED
   return base::StackSamplingProfiler::UnwindersFactory();
 #endif  // ANDROID_UNWINDING_SUPPORTED

diff --git a/chrome/common/profiler/unwind_util.h b/chrome/common/profiler/unwind_util.h
@@ -56,12 +56,7 @@ bool AreUnwindPrerequisitesAvailable(
     version_info::Channel channel,
     UnwindPrerequisitesDelegate* prerequites_delegate = nullptr);
 
-#if BUILDFLAG(IS_ANDROID)
-base::StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactory(
-    bool is_java_name_hashing_enabled);
-#else
 base::StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactory();
-#endif  // BUILDFLAG(IS_ANDROID)
 
 base::StackSamplingProfiler::UnwindersFactory
 CreateLibunwindstackUnwinderFactory();

diff --git a/chrome/gpu/chrome_content_gpu_client.cc b/chrome/gpu/chrome_content_gpu_client.cc
@@ -125,16 +125,10 @@ void ChromeContentGpuClient::PostCompositorThreadCreated(
   // We pass in CreateCoreUnwindersFactory here since it lives in the chrome/
   // layer while TracingSamplerProfiler is outside of chrome/.
   task_runner->PostTask(
-      FROM_HERE, base::BindOnce(&tracing::TracingSamplerProfiler::
-                                    CreateOnChildThreadWithCustomUnwinders,
-#if BUILDFLAG(IS_ANDROID)
-                                base::BindRepeating(
-                                    &CreateCoreUnwindersFactory,
-                                    /*is_java_name_hashing_enabled=*/false)));
-#else
-                                base::BindRepeating(
-                                    &CreateCoreUnwindersFactory)));
-#endif  // BUILDFLAG(IS_ANDROID)
+      FROM_HERE,
+      base::BindOnce(&tracing::TracingSamplerProfiler::
+                         CreateOnChildThreadWithCustomUnwinders,
+                     base::BindRepeating(&CreateCoreUnwindersFactory)));
 }
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)

diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
@@ -1377,11 +1377,7 @@ void ChromeContentRendererClient::PostCompositorThreadCreated(
       FROM_HERE,
       base::BindOnce(&tracing::TracingSamplerProfiler::
                          CreateOnChildThreadWithCustomUnwinders,
-#if BUILDFLAG(IS_ANDROID)
-                     base::BindRepeating(&CreateCoreUnwindersFactory, false)));
-#else
                      base::BindRepeating(&CreateCoreUnwindersFactory)));
-#endif  // BUILDFLAG(IS_ANDROID)
 }
 
 bool ChromeContentRendererClient::RunIdleHandlerWhenWidgetsHidden() {