Skip to content
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

Refactor emulator start and stop functions for clarity and efficiency #22861

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
91d7fc3
Refactor emulator start and stop functions for clarity and efficiency.
jchen351 Nov 15, 2024
d9af6f0
Refactor emulator start and stop functions for clarity and efficiency.
jchen351 Nov 16, 2024
85780c3
Refactor emulator start and stop functions for clarity and efficiency.
jchen351 Nov 16, 2024
afa5e9a
Refactor emulator start and stop functions for clarity and efficiency.
jchen351 Nov 16, 2024
1ceeb24
avd_info
jchen351 Nov 16, 2024
bf977a9
result
jchen351 Nov 16, 2024
a5a45ca
Update tools/python/util/android/android.py
jchen351 Nov 16, 2024
db94c41
Update tools/python/util/android/android.py
jchen351 Nov 16, 2024
5c5b014
Update tools/python/util/android/android.py
jchen351 Nov 16, 2024
a405d62
Update tools/python/util/android/android.py
jchen351 Nov 16, 2024
b9ea99c
Update tools/python/util/android/android.py
jchen351 Nov 16, 2024
5f0f8a8
os
jchen351 Nov 16, 2024
01e3edd
Merge remote-tracking branch 'origin/Cjian/check_emulator_running' in…
jchen351 Nov 16, 2024
4ff05f7
Update tools/python/util/android/android.py
jchen351 Nov 16, 2024
02d619f
Disable bluetooth
jchen351 Nov 18, 2024
f6a9526
Update tools/python/util/android/android.py
jchen351 Nov 18, 2024
2f3ddf1
remove -prop persist.sys.disable_bluetooth=true
jchen351 Nov 18, 2024
b3c7cd4
Check the avd name
jchen351 Nov 18, 2024
fbf5bcc
Update tools/python/util/android/android.py
jchen351 Nov 18, 2024
b98dbd3
litrunner -a
jchen351 Nov 18, 2024
342e42c
Update tools/python/util/android/android.py
jchen351 Nov 21, 2024
a36f12f
Update tools/python/util/android/android.py
jchen351 Nov 21, 2024
3c73a6e
Refactor the code
jchen351 Nov 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 123 additions & 7 deletions tools/python/util/android/android.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update the versions in onnxruntime-extensions and onnxruntime-genai once the changes are checked in.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import collections
import contextlib
import datetime
import os
import signal
import subprocess
import time
Expand Down Expand Up @@ -105,8 +106,13 @@ def _stop_process_with_pid(pid: int):


def start_emulator(
sdk_tool_paths: SdkToolPaths, avd_name: str, extra_args: typing.Optional[typing.Sequence[str]] = None
sdk_tool_paths: SdkToolPaths, avd_name: str, extra_args: typing.Optional[typing.Sequence[str]] = None,
timeout_minutes: int = 20
) -> subprocess.Popen:
jchen351 marked this conversation as resolved.
Show resolved Hide resolved
if is_emulator_running_by_avd(avd_name=avd_name):
raise RuntimeError(
f"An emulator with avd_name{avd_name} is already running. Please close it before starting a new one."
)
with contextlib.ExitStack() as emulator_stack, contextlib.ExitStack() as waiter_stack:
emulator_args = [
sdk_tool_paths.emulator,
Expand All @@ -118,10 +124,12 @@ def start_emulator(
"America/Los_Angeles",
"-no-snapstorage",
"-no-audio",
"-no-snapshot", # No bluetooth
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this parameter was about fast startup from previous run.

https://developer.android.com/studio/run/emulator-snapshots

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is correct. This is not needed for now.

"-no-boot-anim",
"-gpu",
"guest",
"-delay-adb",
"-verbose",
]

# For Linux CIs we must use "-no-window" otherwise you'll get
Expand Down Expand Up @@ -155,9 +163,9 @@ def start_emulator(
waiter_stack.callback(_stop_process, waiter_process)

# poll subprocesses.
# allow 20 minutes for startup as some CIs are slow. TODO: Make timeout configurable if needed.
# allow 20 minutes for startup as some CIs are slow.
sleep_interval_seconds = 10
end_time = datetime.datetime.now() + datetime.timedelta(minutes=20)
end_time = datetime.datetime.now() + datetime.timedelta(minutes=timeout_minutes)

while True:
waiter_ret, emulator_ret = waiter_process.poll(), emulator_process.poll()
Expand Down Expand Up @@ -204,14 +212,122 @@ def start_emulator(

_log.debug(f"sys.boot_completed='{getprop_value}'. Sleeping for {sleep_interval_seconds} before retrying.")
time.sleep(sleep_interval_seconds)

# Verify if the emulator is now running
jchen351 marked this conversation as resolved.
Show resolved Hide resolved
if not is_emulator_running_by_avd(avd_name=avd_name):
raise RuntimeError("Emulator failed to start.")
return emulator_process


def stop_emulator(emulator_proc_or_pid: typing.Union[subprocess.Popen, int]):
def is_emulator_running_by_avd(avd_name: str) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we call these something like check_emulator_running_using_avd_name/check_emulator_running_using_process/check_emulator_running_using_pid?

The 'by' sounds like we could be checked how it was started rather than if it's currently running.

"""
Check if an emulator is running based on the provided AVD name.
:param avd_name: Name of the Android Virtual Device (AVD) to check.
:return: True if an emulator with the given AVD name is running, False otherwise.
"""
try:
# Step 1: List running devices
result = subprocess.check_output(["adb", "devices"], text=True).strip()
_log.info(f"adb devices output:\n{result}")
running_emulators = [line.split("\t")[0] for line in result.splitlines()[1:] if "emulator" in line]

if not running_emulators:
_log.debug("No emulators running.")
return False # No emulators running

# Step 2: Check each running emulator's AVD name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have step 3 of checking getprop sys.boot_completed like we do in start_emulator to make sure they have successfully started?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is necessary, because It is used by start_emulator which calls this:

  1. at the beginning of the function to check if the emulator with the avd name is already running.
  2. at the end of the function to check if the emulator with the avd is running AFTER they have successfully started.

for emulator in running_emulators:
try:
avd_info = subprocess.check_output(["adb", "-s", emulator, "emu", "avd", "name"], text=True).strip().split('\n')[0]
_log.debug(f"AVD name for emulator {emulator}: {avd_info}")
jchen351 marked this conversation as resolved.
Show resolved Hide resolved
if avd_info == avd_name:
return True
except subprocess.SubprocessError:
_log.warning(f"Error checking AVD name for emulator: {emulator}")
continue # Skip if there's an issue querying a specific emulator
_log.warning(f"No emulator running with AVD name: {avd_name}")
jchen351 marked this conversation as resolved.
Show resolved Hide resolved
return False # No matching AVD name found
except subprocess.SubprocessError as e:
_log.warning(f"Error checking emulator status: {e}")
return False


def is_emulator_running_by_proc(emulator_proc: subprocess.Popen) -> bool:
"""Check if the emulator process is running based on a Popen instance."""
return emulator_proc.poll() is None


def is_emulator_running_by_pid(emulator_pid: int) -> bool:
"""Check if the emulator process is running based on PID."""
try:
os.kill(emulator_pid, 0) # Signal 0 checks process existence
Fixed Show fixed Hide fixed
return True
except OSError:
return False


def stop_emulator_by_proc(emulator_proc: subprocess.Popen, timeout: int = 120):
jchen351 marked this conversation as resolved.
Show resolved Hide resolved
"""
Stops the emulator process using a subprocess.Popen instance.
:param emulator_proc: The emulator process as a subprocess.Popen instance.
:param timeout: Maximum time (in seconds) to wait for the emulator to stop.
"""
if not is_emulator_running_by_proc(emulator_proc):
_log.warning("The specified emulator process is not running.")
return

_log.info("Stopping emulator using subprocess.Popen instance.")
_stop_process(emulator_proc)

# Wait for the process to stop
interval = 5
end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout)

while is_emulator_running_by_proc(emulator_proc):
if datetime.datetime.now() > end_time:
raise RuntimeError(f"Failed to stop the emulator within the specified timeout = {timeout} seconds.")
_log.debug("Emulator still running. Checking again in 5 seconds...")
time.sleep(interval)

_log.info("Emulator stopped successfully.")


def stop_emulator_by_pid(emulator_pid: int, timeout: int = 120):
"""
Stops the emulator process using a PID.
:param emulator_pid: The emulator process PID.
:param timeout: Maximum time (in seconds) to wait for the emulator to stop.
"""
if not is_emulator_running_by_pid(emulator_pid):
_log.warning(f"No emulator process with PID {emulator_pid} is currently running.")
return

_log.info(f"Stopping emulator with PID: {emulator_pid}")
_stop_process_with_pid(emulator_pid)

# Wait for the process to stop
interval = 5
end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout)

while is_emulator_running_by_pid(emulator_pid):
if datetime.datetime.now() > end_time:
raise RuntimeError(
f"Failed to stop the emulator with PID {emulator_pid} within the specified timeout = {timeout} seconds."
)
_log.debug("Emulator still running. Checking again in 5 seconds...")
time.sleep(interval)

_log.info("Emulator stopped successfully.")


def stop_emulator(emulator_proc_or_pid: typing.Union[subprocess.Popen, int], timeout: int = 120):
"""
Stops the emulator process, checking its running status before and after stopping.
:param emulator_proc_or_pid: The emulator process (subprocess.Popen) or PID (int).
:param timeout: Maximum time (in seconds) to wait for the emulator to stop.
"""
if isinstance(emulator_proc_or_pid, subprocess.Popen):
_stop_process(emulator_proc_or_pid)
stop_emulator_by_proc(emulator_proc_or_pid, timeout)
elif isinstance(emulator_proc_or_pid, int):
_stop_process_with_pid(emulator_proc_or_pid)
stop_emulator_by_pid(emulator_proc_or_pid, timeout)
else:
raise ValueError("Expected either a PID or subprocess.Popen instance.")
Loading