Skip to content

Commit 6acc0ec

Browse files
darrenwu-gitV8 LUCI CQ
authored andcommitted
Add ChromiumPgoProbe for Android PGO profile collection
This commit introduces `ChromiumPgoProbe`, a new probe designed to collect Profile Guided Optimization (PGO) data from Chromium-based browsers running on Android devices. Key features and changes: - Implemented `ChromiumPgoProbe` to manage PGO profile dumping, downloading, and cleanup on Android. - Added the `--remote-allow-origins=*` flag to `ChromiumPgoProbe` to ensure DevTools is opened for PGO operations. - Added DevTools communication capabilities to the Android ADB platform layer to trigger PGO dumps and manage profiles. This includes new methods for sending DevTools commands, listing PGO files, and cleaning PGO directories. - Updated the base `Platform` interface and its Linux/macOS implementations to reflect new PGO-related methods, currently raising `NotImplementedError` for non-Android platforms. - Registered `ChromiumPgoProbe` for general use. - Added an `expect_android` validation method in the base `Probe` class. - Included HJSON documentation for configuring `ChromiumPgoProbe`, clarifying the optional `remote_pgo_dir_template` parameter and its default value. This probe facilitates the collection of PGO profiles, which are essential for optimizing Chrome's performance. Bug: 415130383 Change-Id: I1ff5f657fd4d579bb515fdadfa48684e456e7f74 Reviewed-on: https://chromium-review.googlesource.com/c/crossbench/+/6545639 Auto-Submit: Darren Wu <[email protected]> Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Darren Wu <[email protected]>
1 parent 76223ae commit 6acc0ec

File tree

11 files changed

+418
-4
lines changed

11 files changed

+418
-4
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
probes: {
3+
chromium_pgo: {
4+
// (Optional) Root path for the PGO profile directories on the target device.
5+
// The browser package name and pgo directory path will be appended to this path.
6+
// Example: org.chromium.chrome/cache/pgo_profiles
7+
remote_pgo_root_path: /data_mirror/data_ce/null/0/
8+
}
9+
}
10+
}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Copyright 2025 The Chromium Authors
2+
# Use of this source code is governed by a BSD-style license that can be
3+
# found in the LICENSE file.
4+
5+
from __future__ import annotations
6+
7+
import json
8+
import logging
9+
from typing import TYPE_CHECKING, Any
10+
11+
import websocket
12+
13+
if TYPE_CHECKING:
14+
from crossbench.plt.base import Platform
15+
16+
17+
class DevToolsClient:
18+
"""Manages communication with the Chrome DevTools Protocol."""
19+
20+
def __init__(self,
21+
platform: Platform,
22+
requested_local_port: int = 0,
23+
remote_devtools_identifier: str = "chrome_devtools_remote"):
24+
self._platform: Platform = platform
25+
self._requested_local_port: int = requested_local_port
26+
self._remote_devtools_identifier: str = remote_devtools_identifier
27+
self._ws: websocket.WebSocket | None = None
28+
self._devtools_port: int = 0
29+
30+
def connect(self) -> None:
31+
"""Establishes a WebSocket connection to the DevTools service."""
32+
if self._ws and self._ws.connected:
33+
return
34+
try:
35+
self._devtools_port = self._platform.forward_devtools_port(
36+
local_port=self._requested_local_port,
37+
remote_identifier=self._remote_devtools_identifier)
38+
self._ws = websocket.WebSocket()
39+
self._ws.connect(
40+
f"ws://localhost:{self._devtools_port}/devtools/browser/")
41+
logging.debug("DevTools connected: ws://localhost:%s/devtools/browser/",
42+
self._devtools_port)
43+
except (websocket.WebSocketException, ConnectionRefusedError,
44+
TimeoutError) as e:
45+
logging.error("DevTools connection error: %s", e)
46+
self._disconnect_internal()
47+
raise
48+
except Exception as e:
49+
logging.error("Unexpected error during DevTools connection: %s", e)
50+
self._disconnect_internal()
51+
raise
52+
53+
def _disconnect_internal(self) -> None:
54+
if self._ws and self._ws.connected:
55+
try:
56+
self._ws.close()
57+
except websocket.WebSocketException as e:
58+
logging.warning("Error closing DevTools WebSocket: %s", e)
59+
self._ws = None
60+
if self._devtools_port:
61+
try:
62+
self._platform.stop_port_forward(self._devtools_port)
63+
except Exception as e: # pylint: disable=broad-except
64+
# Best effort to remove forwarding, log if it fails but don't crash
65+
logging.warning(
66+
"Error removing DevTools port forwarding for port %s: %s",
67+
self._devtools_port, e)
68+
self._devtools_port = 0
69+
70+
def disconnect(self) -> None:
71+
"""Closes the WebSocket connection and removes port forwarding."""
72+
self._disconnect_internal()
73+
logging.debug("DevTools disconnected")
74+
75+
def send_command(self, command_payload: dict[str, Any]) -> bool:
76+
"""Sends a command to DevTools and checks the response ID.
77+
78+
Args:
79+
command_payload: The command payload to send. Must include an 'id'.
80+
81+
Returns:
82+
True if the command was sent successfully and the response ID matches,
83+
False otherwise.
84+
"""
85+
if not self._ws or not self._ws.connected:
86+
logging.error("DevTools is not connected. Cannot send command.")
87+
return False
88+
89+
expected_id = command_payload.get("id")
90+
if expected_id is None:
91+
logging.error("DevTools command requires an 'id' in the payload.")
92+
return False
93+
94+
try:
95+
self._ws.send(json.dumps(command_payload).encode("utf-8"))
96+
data = self._ws.recv()
97+
response = json.loads(data)
98+
return response.get("id") == expected_id
99+
except (websocket.WebSocketException, ConnectionRefusedError,
100+
TimeoutError) as e:
101+
logging.error("DevTools communication error: %s", e)
102+
return False
103+
except json.JSONDecodeError as e:
104+
logging.error("Error decoding JSON response from DevTools: %s", e)
105+
return False
106+
107+
def __enter__(self) -> DevToolsClient:
108+
self.connect()
109+
return self
110+
111+
def __exit__(self, exc_type, exc_val, exc_tb) -> None:
112+
self.disconnect()

crossbench/plt/android_adb.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from snippet_uiautomator import uiautomator
1717
from typing_extensions import override
1818

19+
from crossbench.flags.base import Flags, FlagsData
1920
from crossbench import path as pth
2021
from crossbench.parse import NumberParser
2122
from crossbench.plt.arch import MachineArch
@@ -307,9 +308,19 @@ def unroot(self) -> None:
307308
def devices(self) -> dict[str, dict[str, str]]:
308309
return adb_devices(self._host_platform, self._adb_bin)
309310

310-
def forward(self, local: int, remote: int, protocol: str = "tcp") -> int:
311-
stdout = self._adb_stdout("forward", f"{protocol}:{local}",
312-
f"{protocol}:{remote}").strip()
311+
def forward(self,
312+
local: int,
313+
remote: int | str,
314+
local_protocol: str = "tcp",
315+
remote_protocol: str = "tcp",
316+
flags_data: FlagsData = None) -> int:
317+
cmd_args: list[Any] = ["forward"]
318+
if flags_data:
319+
parsed_flags = Flags(flags_data)
320+
cmd_args.extend(list(parsed_flags))
321+
cmd_args.append(f"{local_protocol}:{local}")
322+
cmd_args.append(f"{remote_protocol}:{remote}")
323+
stdout = self._adb_stdout(*cmd_args).strip()
313324
if not stdout:
314325
used_ports = self._adb_stdout("forward", "--list")
315326
raise ValueError(
@@ -673,11 +684,24 @@ def sh_stdout_bytes(self,
673684
def port_forward(self, local_port: int, remote_port: int) -> int:
674685
local_port = NumberParser.positive_zero_int(local_port, "local_port")
675686
remote_port = NumberParser.port_number(remote_port, "remote_port")
676-
local_port = self.adb.forward(local_port, remote_port, protocol="tcp")
687+
local_port = self.adb.forward(local_port, remote_port)
677688
logging.debug("Forwarded Remote Port: %s:%s <= %s:%s",
678689
self._host_platform.name, local_port, self, remote_port)
679690
return local_port
680691

692+
@override
693+
def forward_devtools_port(self, local_port: int,
694+
remote_identifier: str) -> int:
695+
local_port = NumberParser.positive_zero_int(local_port, "local_port")
696+
local_port = self.adb.forward(
697+
local=local_port,
698+
remote=remote_identifier,
699+
local_protocol="tcp",
700+
remote_protocol="localabstract")
701+
logging.debug("Forwarded DevTools Port: %s:%s <= %s:%s",
702+
self._host_platform.name, local_port, self, remote_identifier)
703+
return local_port
704+
681705
@override
682706
def stop_port_forward(self, local_port: int) -> None:
683707
self.adb.forward_remove(local_port, protocol="tcp")

crossbench/plt/base.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,26 @@ def port_forward(self, local_port: int, remote_port: int) -> int:
586586
self.assert_is_local()
587587
return local_port
588588

589+
def forward_devtools_port(self, local_port: int,
590+
remote_identifier: str) -> int:
591+
"""Forwards a DevTools debugging port from a remote target to a local port.
592+
593+
Args:
594+
local_port: The local port number to forward to. If 0, a free
595+
port will be chosen by the system.
596+
remote_identifier: A string identifying the remote DevTools socket or
597+
service. For Android, this is typically a
598+
localabstract socket name like
599+
"chrome_devtools_remote".
600+
For other platforms, it might be a remote port number
601+
or other service identifier.
602+
603+
Returns:
604+
The local port number that was actually used for forwarding.
605+
"""
606+
raise NotImplementedError(
607+
f"forward_devtools_port not implemented for {self}")
608+
589609
def stop_port_forward(self, local_port: int) -> None:
590610
# TODO: Migrate forwarding methods to custom PortManager
591611
del local_port
@@ -838,6 +858,9 @@ def file_size(self, path: pth.AnyPathLike) -> int:
838858
# TODO: support remotely
839859
return self.local_path(path).stat().st_size
840860

861+
def last_modified(self, path: pth.AnyPathLike) -> float:
862+
return self.local_path(path).stat().st_mtime
863+
841864
def sh_stdout(self,
842865
*args: CmdArg,
843866
shell: bool = False,

crossbench/plt/macos.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,3 +583,10 @@ def is_port_used(self, port: int) -> bool:
583583
# This is a semi-ideal solution as it creates a temporary local server.
584584
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
585585
return s.connect_ex(("localhost", port)) == 0
586+
587+
@override
588+
def last_modified(self, path: pth.AnyPathLike) -> float:
589+
if self.is_local:
590+
return super().last_modified(path)
591+
# Get seconds since epoch
592+
return float(self.sh_stdout("stat", "-f", "%m", self.path(path)))

crossbench/plt/posix.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,13 @@ def user_id(self) -> int:
487487
return super().user_id()
488488
return int(self.sh_stdout("id", "-u").strip())
489489

490+
@override
491+
def last_modified(self, path: pth.AnyPathLike) -> float:
492+
if self.is_local:
493+
return super().last_modified(path)
494+
# Get seconds since epoch
495+
return float(self.sh_stdout("stat", "-c", "%Y", self.path(path)))
496+
490497

491498
class RemotePosixEnviron(Environ):
492499

crossbench/probes/all.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from crossbench.probes.android_logcat import LogcatAndroidProbe
1010
from crossbench.probes.chrome_histograms import ChromeHistogramsProbe
11+
from crossbench.probes.chromium_pgo_probe import ChromiumPgoProbe
1112
from crossbench.probes.debugger import DebuggerProbe
1213
from crossbench.probes.downloads import DownloadsProbe
1314
from crossbench.probes.dtrace import DTraceProbe
@@ -89,6 +90,7 @@
8990
GENERAL_PURPOSE_PROBES: tuple[Type[Probe], ...] = (
9091
BrowserProfilingProbe,
9192
ChromeHistogramsProbe,
93+
ChromiumPgoProbe,
9294
DebuggerProbe,
9395
DownloadsProbe,
9496
DTraceProbe,

0 commit comments

Comments
 (0)