Skip to content

Commit

Permalink
Extract class ReportPageProcessesPolicy
Browse files Browse the repository at this point in the history 
Reporting page processes is in OomScorePolicyChromeOS to share some
code, but eventually the shared code is little. Extract class
ReportPageProcessesPolicy to make it easier to change.

Bug: b/304679315
Change-Id: Id7c77d777d4242313560f44dbb188f1e9da9aca3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4924187
Commit-Queue: Vovo Yang <vovoy@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209644}
  • Loading branch information
Vovo Yang authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent 0110602 commit 70ab0a7
Show file tree
Hide file tree
Showing 9 changed files with 430 additions and 240 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5784,6 +5784,8 @@ static_library("browser") {
"obsolete_system/obsolete_system_stub.cc",
"performance_manager/policies/oom_score_policy_chromeos.cc",
"performance_manager/policies/oom_score_policy_chromeos.h",
"performance_manager/policies/report_page_processes_policy.cc",
"performance_manager/policies/report_page_processes_policy.h",
"policy/messaging_layer/util/manual_test_heartbeat_event.cc",
"policy/messaging_layer/util/manual_test_heartbeat_event.h",
"policy/messaging_layer/util/manual_test_heartbeat_event_factory.cc",
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@

#if BUILDFLAG(IS_CHROMEOS)
#include "chrome/browser/performance_manager/policies/oom_score_policy_chromeos.h"
#include "chrome/browser/performance_manager/policies/report_page_processes_policy.h"
#endif // BUILDFLAG(IS_CHROMEOS)

#if BUILDFLAG(ENABLE_EXTENSIONS)
Expand Down Expand Up @@ -137,13 +138,19 @@ void ChromeBrowserMainExtraPartsPerformanceManager::CreatePoliciesAndDecorators(
#if BUILDFLAG(IS_CHROMEOS_LACROS)
graph->PassToGraph(std::make_unique<
performance_manager::policies::OomScorePolicyChromeOS>());
graph->PassToGraph(
std::make_unique<
performance_manager::policies::ReportPageProcessesPolicy>());
#elif BUILDFLAG(IS_CHROMEOS_ASH)
if (base::FeatureList::IsEnabled(
performance_manager::features::
kAshUrgentDiscardingFromPerformanceManager)) {
graph->PassToGraph(
std::make_unique<
performance_manager::policies::OomScorePolicyChromeOS>());
graph->PassToGraph(
std::make_unique<
performance_manager::policies::ReportPageProcessesPolicy>());
}
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)

Expand Down
127 changes: 14 additions & 113 deletions chrome/browser/performance_manager/policies/oom_score_policy_chromeos.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,12 @@
#include <algorithm>
#include <set>

#include "base/process/memory.h" // For AdjustOOMScore
#include "base/process/memory.h"
#include "components/performance_manager/public/graph/frame_node.h"
#include "components/performance_manager/public/graph/process_node.h"
#include "content/public/browser/browser_thread.h" // For GetUIThreadTaskRunner
#include "content/public/common/content_constants.h" // For kLowestRendererOomScore
#include "content/public/common/content_constants.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "chromeos/ash/components/dbus/resourced/resourced_client.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chromeos/crosapi/mojom/resource_manager.mojom.h"
#include "chromeos/lacros/lacros_service.h"
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)

namespace performance_manager {
namespace policies {
namespace performance_manager::policies {

namespace {

Expand All @@ -33,55 +22,6 @@ namespace {
constexpr base::TimeDelta kOomScoresAssignmentMinimalInterval =
base::Seconds(10);

// The minimal interval of background pids report. The throttling is to reduce
// the overhead of the pids reporting. When there is no memory pressure, the
// reported pid list is not used. Under memory pressure, the background pids
// are used to calculate the background Chrome memory usage. When the memory
// pressure is higher, it requires larger amount of background memory usage to
// change the low memory handling policy (whether to avoid killing perceptible
// apps) [1]. So small deviation of the background memory usage caused by
// out-of-dated pid list would only make the policy change a little bit earlier
// or later.
//
// [1]:
// https://chromium-review.googlesource.com/c/chromiumos/platform2/+/4889461/9/resourced/src/memory.rs#665
constexpr base::TimeDelta kBackgroundPidsReportMinimalInterval =
base::Seconds(3);

void ReportBackgroundProcessesOnUIThread(
const std::vector<base::ProcessId>& pids) {
#if BUILDFLAG(IS_CHROMEOS_ASH)
ash::ResourcedClient* client = ash::ResourcedClient::Get();
if (client) {
client->ReportBackgroundProcesses(ash::ResourcedClient::Component::kAsh,
pids);
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_CHROMEOS_LACROS)
chromeos::LacrosService* service = chromeos::LacrosService::Get();
// Check LacrosService availability to avoid crashing
// lacros_chrome_browsertests.
if (!service || !service->IsAvailable<crosapi::mojom::ResourceManager>()) {
LOG(ERROR) << "ResourceManager is not available";
return;
}

int resource_manager_version =
service->GetInterfaceVersion<crosapi::mojom::ResourceManager>();
if (resource_manager_version <
int{crosapi::mojom::ResourceManager::MethodMinVersions::
kReportBackgroundProcessesMinVersion}) {
LOG(WARNING) << "Resource Manager version " << resource_manager_version
<< " does not support reporting background processes.";
return;
}

service->GetRemote<crosapi::mojom::ResourceManager>()
->ReportBackgroundProcesses(pids);
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
}

} // namespace

OomScorePolicyChromeOS::OomScorePolicyChromeOS() = default;
Expand Down Expand Up @@ -119,35 +59,19 @@ void OomScorePolicyChromeOS::OnTypeChanged(const PageNode* page_node,

void OomScorePolicyChromeOS::HandlePageNodeEventsThrottled() {
base::TimeTicks now = base::TimeTicks::Now();
bool oom_scores_assignment = false;
bool background_pids_report = false;

if (now - last_oom_scores_assignment_ > kOomScoresAssignmentMinimalInterval) {
last_oom_scores_assignment_ = now;
oom_scores_assignment = true;
}
if (now - last_background_pids_report_ >
kBackgroundPidsReportMinimalInterval) {
last_background_pids_report_ = now;
background_pids_report = true;
HandlePageNodeEvents();
}

HandlePageNodeEvents(oom_scores_assignment, background_pids_report);
}

void OomScorePolicyChromeOS::HandlePageNodeEvents(bool oom_scores_assignment,
bool background_pids_report) {
if (!oom_scores_assignment && !background_pids_report) {
return;
}

void OomScorePolicyChromeOS::HandlePageNodeEvents() {
PageDiscardingHelper* discarding_helper =
PageDiscardingHelper::GetFromGraph(graph_);

std::vector<const PageNode*> page_nodes = graph_->GetAllPageNodes();

std::vector<PageNodeSortProxy> candidates;
std::vector<PageNodeSortProxy> background_candidates;

for (const auto* page_node : page_nodes) {
PageDiscardingHelper::CanDiscardResult can_discard_result =
Expand All @@ -158,32 +82,17 @@ void OomScorePolicyChromeOS::HandlePageNodeEvents(bool oom_scores_assignment,
bool is_protected = (can_discard_result ==
PageDiscardingHelper::CanDiscardResult::kProtected);
bool is_visible = page_node->IsVisible();
if (oom_scores_assignment) {
candidates.emplace_back(page_node, is_marked, is_visible, is_protected,
page_node->GetTimeSinceLastVisibilityChange());
}
if (background_pids_report && !is_marked && !is_protected) {
background_candidates.emplace_back(
page_node, is_marked, is_visible, is_protected,
page_node->GetTimeSinceLastVisibilityChange());
}
candidates.emplace_back(page_node, is_marked, is_visible, is_protected,
page_node->GetTimeSinceLastVisibilityChange());
}

if (oom_scores_assignment) {
// Sorts with descending importance.
std::sort(candidates.begin(), candidates.end(),
[](const PageNodeSortProxy& lhs, const PageNodeSortProxy& rhs) {
return rhs < lhs;
});
// Sorts with descending importance.
std::sort(candidates.begin(), candidates.end(),
[](const PageNodeSortProxy& lhs, const PageNodeSortProxy& rhs) {
return rhs < lhs;
});

oom_score_map_ = DistributeOomScore(candidates);
}

if (background_pids_report) {
std::vector<base::ProcessId> background_pids =
GetUniquePids(background_candidates);
ReportBackgroundProcesses(std::move(background_pids));
}
oom_score_map_ = DistributeOomScore(candidates);
}

OomScorePolicyChromeOS::ProcessScoreMap
Expand Down Expand Up @@ -260,12 +169,4 @@ int OomScorePolicyChromeOS::GetCachedOomScore(base::ProcessHandle pid) {
return it->second;
}

void OomScorePolicyChromeOS::ReportBackgroundProcesses(
std::vector<base::ProcessId> pids) {
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&ReportBackgroundProcessesOnUIThread, std::move(pids)));
}

} // namespace policies
} // namespace performance_manager
} // namespace performance_manager::policies
33 changes: 7 additions & 26 deletions chrome/browser/performance_manager/policies/oom_score_policy_chromeos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,21 @@

#include <vector>

#include "base/process/process_handle.h" // For ProcessId
#include "base/containers/flat_map.h"
#include "base/process/process_handle.h"
#include "base/sequence_checker.h"
#include "base/time/time.h"
#include "chrome/browser/performance_manager/policies/page_discarding_helper.h"
#include "components/performance_manager/public/graph/graph.h"
#include "components/performance_manager/public/graph/page_node.h"
#include "components/performance_manager/public/graph/system_node.h"

namespace performance_manager {

namespace policies {
namespace performance_manager::policies {

// Assigning oom score adj to renderer processes. Process with lowest oom score
// adj is the last to be killed by Linux oom killer. The more important process
// would be assigned lower oom score adj. See the following web page for more
// explanation on Linux oom score adj(adjust).
// [1]: https://man7.org/linux/man-pages/man1/choom.1.html
//
// This class is also responsible to send the background process list to the
// resource manager daemon (resourced). The background process list is used to
// estimate the memory usage of the background processes. Based on the
// background Chrome memory usage, resourced determines when to release memory
// from Chrome or VMs or container. When there are a lot of background processes
// in Ash Chrome, resourced would avoid killing perceptible apps in VMs or
// container.
class OomScorePolicyChromeOS : public GraphOwned,
public PageNode::ObserverDefaultImpl {
public:
Expand All @@ -53,21 +43,12 @@ class OomScorePolicyChromeOS : public GraphOwned,

protected:
// These members are protected for testing.
void HandlePageNodeEvents(bool oom_scores_assignment,
bool background_pids_report);
void HandlePageNodeEvents();

// Returns the cached oom score adj. If the pid is not cached, returns -1 (a
// value not in the valid oom score adj range for renderer processes).
int GetCachedOomScore(base::ProcessId pid);

// Reports the background process list to the resource manager daemon
// (resourced). Based on the background process list, resourced determines
// when to release memory from Chrome or VMs or containers.
// It's virtual for testing.
virtual void ReportBackgroundProcesses(std::vector<base::ProcessId> pids);

raw_ptr<Graph> graph_ = nullptr;

private:
// Cache OOM scores in memory.
using ProcessScoreMap = base::flat_map<base::ProcessId, int>;
Expand All @@ -85,7 +66,8 @@ class OomScorePolicyChromeOS : public GraphOwned,
const std::vector<PageNodeSortProxy>& candidates);

base::TimeTicks last_oom_scores_assignment_ = base::TimeTicks::Now();
base::TimeTicks last_background_pids_report_ = base::TimeTicks::Now();

raw_ptr<Graph> graph_ = nullptr;

// Map maintaining the process handle - oom_score mapping.
ProcessScoreMap oom_score_map_;
Expand All @@ -95,7 +77,6 @@ class OomScorePolicyChromeOS : public GraphOwned,
base::WeakPtrFactory<OomScorePolicyChromeOS> weak_factory_{this};
};

} // namespace policies
} // namespace performance_manager
} // namespace performance_manager::policies

#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_POLICIES_OOM_SCORE_POLICY_CHROMEOS_H_

0 comments on commit 70ab0a7

Please sign in to comment.