-
Notifications
You must be signed in to change notification settings - Fork 187
android: Cobalt APK comparison script #9039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new Python script for comparing memory and CPU performance between two Cobalt APKs. The script is well-structured and provides a useful utility for performance analysis. For easier dependency management, consider adding a requirements.txt file listing pandas and matplotlib. My review focuses on improving robustness, maintainability, and adherence to Python style guides. Key suggestions include fixing a critical bug that would cause the script to crash, making the script more portable by removing hardcoded paths, improving the reliability of CPU monitoring, and addressing several style guide violations such as missing docstrings and excessive line lengths. I've also pointed out some unnecessary delays that could be removed to make the script run faster.
| print(f"Error parsing meminfo: {e}") | ||
| return None | ||
|
|
||
| def monitor_cpu(pid): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsing the output of top is notoriously fragile as its format can vary across different Android versions and device manufacturers. For example, the CPU column header S[%CPU] might not be consistent. For a more robust solution, consider parsing the /proc/<pid>/stat file to calculate CPU usage. This involves reading CPU time values at two points in time and calculating the percentage against the total CPU time elapsed. While more complex, it's a standard and reliable method on Linux-based systems.
| ] | ||
| run_adb_command(launch_command) | ||
| print("Waiting for app to start...") | ||
| time.sleep(10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a hardcoded 10-second sleep to wait for the app to start. The code that follows in run_test_on_apk already polls for the process PID to appear. This fixed-delay sleep is redundant with the polling mechanism and can be safely removed to make the script faster and more efficient. The polling loop will handle waiting for the app to be ready.
Script to compare memory and CPU across 2 apks.
Requires installing pip modules 'pandas' and 'matplotlib' to run.
$ tools/cobalt_compare_apk_mem_and_cpu.py --help
usage: cobalt_compare_apk_mem_and_cpu.py [-h] [--apk1 APK1] [--apk2 APK2] [--duration DURATION] [--outdir OUTDIR] [--url URL]
Monitor and Compare Cobalt APKs.
options:
-h, --help show this help message and exit
--apk1 APK1 Path to the first Cobalt APK
--apk2 APK2 Path to the second Cobalt APK
--duration DURATION Duration to monitor each APK in seconds
--outdir OUTDIR Directory to save plots and CSV files
--url URL YouTube URL to launch
Bug: 483129786