Skip to content

Commit

Permalink
Revert "Widen CPU affinity set." (#53274)
Browse files Browse the repository at this point in the history
Reverts #53136

b/345642546 shows some massive performance regressions on this patch. i'll rework it so the conditions are only loosened on phones with one fast core.
  • Loading branch information
jonahwilliams authored Jun 7, 2024
1 parent 3a92452 commit 69a4041
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 13 deletions.
4 changes: 0 additions & 4 deletions fml/cpu_affinity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ CPUSpeedTracker::CPUSpeedTracker(std::vector<CpuIndexAndSpeed> data)
}
if (data.speed == min_speed_value) {
efficiency_.push_back(data.index);
} else {
not_efficiency_.push_back(data.index);
}
}

Expand All @@ -79,8 +77,6 @@ const std::vector<size_t>& CPUSpeedTracker::GetIndices(
return efficiency_;
case CpuAffinity::kNotPerformance:
return not_performance_;
case CpuAffinity::kNotEfficiency:
return not_efficiency_;
}
}

Expand Down
4 changes: 0 additions & 4 deletions fml/cpu_affinity.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ enum class CpuAffinity {

/// @brief Request affinity for all non-performance cores.
kNotPerformance,

/// @brief Request affinity for all non-efficiency cores.
kNotEfficiency,
};

/// @brief Request count of efficiency cores.
Expand Down Expand Up @@ -82,7 +79,6 @@ class CPUSpeedTracker {
std::vector<size_t> efficiency_;
std::vector<size_t> performance_;
std::vector<size_t> not_performance_;
std::vector<size_t> not_efficiency_;
};

/// @note Visible for testing.
Expand Down
3 changes: 0 additions & 3 deletions fml/cpu_affinity_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ TEST(CpuAffinity, NormalSlowMedFastCores) {
ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotPerformance).size(), 2u);
ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotPerformance)[0], 0u);
ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotPerformance)[1], 1u);
ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotEfficiency).size(), 2u);
ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotEfficiency)[0], 1u);
ASSERT_EQ(tracker.GetIndices(CpuAffinity::kNotEfficiency)[1], 2u);
}

TEST(CpuAffinity, NoCpuData) {
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/android/android_shell_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ static void AndroidPlatformThreadConfigSetter(
break;
}
case fml::Thread::ThreadPriority::kDisplay: {
fml::RequestAffinity(fml::CpuAffinity::kNotEfficiency);
fml::RequestAffinity(fml::CpuAffinity::kPerformance);
if (::setpriority(PRIO_PROCESS, 0, -1) != 0) {
FML_LOG(ERROR) << "Failed to set UI task runner priority";
}
break;
}
case fml::Thread::ThreadPriority::kRaster: {
fml::RequestAffinity(fml::CpuAffinity::kNotEfficiency);
fml::RequestAffinity(fml::CpuAffinity::kPerformance);
// Android describes -8 as "most important display threads, for
// compositing the screen and retrieving input events". Conservatively
// set the raster thread to slightly lower priority than it.
Expand Down

0 comments on commit 69a4041

Please sign in to comment.