Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

Speed up emulator port allocation #46

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
29 changes: 13 additions & 16 deletions swarmer/src/main/kotlin/com/gojuno/swarmer/Emulators.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,14 @@ fun startEmulators(
connectedAdbDevices: () -> Observable<Set<AdbDevice>> = ::connectedAdbDevices,
createAvd: (args: Commands.Start) -> Observable<Unit> = ::createAvd,
applyConfig: (args: Commands.Start) -> Observable<Unit> = ::applyConfig,
emulator: (args: Commands.Start) -> String = ::emulatorBinary,
emulatorCmd: (args: Commands.Start) -> String = ::emulatorBinary,
findAvailablePortsForNewEmulator: () -> Observable<Pair<Int, Int>> = ::findAvailablePortsForNewEmulator,
startEmulatorProcess: (List<String>, Commands.Start) -> Observable<Notification> = ::startEmulatorProcess,
waitForEmulatorToStart: (Commands.Start, () -> Observable<Set<AdbDevice>>, Observable<Notification>, Pair<Int, Int>) -> Observable<Emulator> = ::waitForEmulatorToStart,
waitForEmulatorToFinishBoot: (Emulator, Commands.Start) -> Observable<Emulator> = ::waitForEmulatorToFinishBoot
) {
val startTime = System.nanoTime()

// Sometimes on Linux "emulator -verbose -avd" does not print serial id of started emulator,
// so by allocating ports manually we know which serial id emulator will have.
val availablePortsSemaphore = Semaphore(1)

val startedEmulators = connectedAdbDevices()
.doOnNext { log("Already running emulators: $it") }
.flatMap {
Expand All @@ -54,12 +50,11 @@ fun startEmulators(
args = command,
createAvd = createAvd,
applyConfig = applyConfig,
availablePortsSemaphore = availablePortsSemaphore,
findAvailablePortsForNewEmulator = findAvailablePortsForNewEmulator,
startEmulatorProcess = startEmulatorProcess,
waitForEmulatorToStart = waitForEmulatorToStart,
connectedAdbDevices = connectedAdbDevices,
emulator = emulator,
emulatorCmd = emulatorCmd,
waitForEmulatorToFinishBoot = waitForEmulatorToFinishBoot
)
}
Expand Down Expand Up @@ -87,29 +82,26 @@ private fun startEmulator(
args: Commands.Start,
createAvd: (args: Commands.Start) -> Observable<Unit>,
applyConfig: (args: Commands.Start) -> Observable<Unit>,
availablePortsSemaphore: Semaphore,
findAvailablePortsForNewEmulator: () -> Observable<Pair<Int, Int>>,
startEmulatorProcess: (List<String>, Commands.Start) -> Observable<Notification>,
waitForEmulatorToStart: (Commands.Start, () -> Observable<Set<AdbDevice>>, Observable<Notification>, Pair<Int, Int>) -> Observable<Emulator>,
connectedAdbDevices: () -> Observable<Set<AdbDevice>> = ::connectedAdbDevices,
emulator: (Commands.Start) -> String,
emulatorCmd: (Commands.Start) -> String,
waitForEmulatorToFinishBoot: (Emulator, Commands.Start) -> Observable<Emulator>
): Observable<Emulator> =
createAvd(args)
.flatMap { applyConfig(args) }
.map { availablePortsSemaphore.acquire() }
.flatMap { findAvailablePortsForNewEmulator() }
.doOnNext { log("Ports for emulator ${args.emulatorName}: ${it.first}, ${it.second}.") }
.flatMap { ports ->
startEmulatorProcess(
// Unix only, PR welcome.
listOf(sh, "-c", "${emulator(args)} ${if (args.verbose) "-verbose" else ""} -avd ${args.emulatorName} -ports ${ports.first},${ports.second} ${args.emulatorStartOptions.joinToString(" ")} &"),
listOf(sh, "-c", "${emulatorCmd(args)} ${if (args.verbose) "-verbose" else ""} -avd ${args.emulatorName} -ports ${ports.first},${ports.second} ${args.emulatorStartOptions.joinToString(" ")} &"),
args
).let { process ->
waitForEmulatorToStart(args, connectedAdbDevices, process, ports)
}
}
.map { emulator -> availablePortsSemaphore.release().let { emulator } }
.flatMap { emulator ->
when (args.redirectLogcatTo) {
null -> Observable.just(emulator)
Expand Down Expand Up @@ -219,17 +211,22 @@ private fun emulatorBinary(args: Commands.Start): String =
emulator
}

private val usedPorts: MutableList<Int> = mutableListOf()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is scary change, I actually used something like this in the beggining, but then figured that it breaks when something else starts emulators outside of Swarmer

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I might be wrong here, if this is just the used ports it should be fine

Copy link
Author

Choose a reason for hiding this comment

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

I think there is always a small risk if something starts emulators outside of Swarmer that can't be avoided. If a emulator would start after findAvailablePortsForNewEmulator but before Swarmer starts the emulator. Not sure if there is a way to avoid possible port conflicts in that case. But that is a small window so I think we should be fine.

/**
* Sometimes on Linux "emulator -verbose -avd" does not print serial id of started emulator,
* so by allocating ports manually we know which serial id emulator will have.
*/
private fun findAvailablePortsForNewEmulator(): Observable<Pair<Int, Int>> = connectedAdbDevices()
.map { it.filter { it.isEmulator } }
.map {
if (it.isEmpty()) {
5554
} else {
synchronized(usedPorts) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use ConcurrentHashMap().toSet() so you have thread-safe MutableSet without need for synchronization

Copy link
Author

Choose a reason for hiding this comment

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

How would the usage look like? We basically need to read the entire list and then write to it before the next thread is allowed to read it. Not sure if ConcurrentHashMap can do that. But a AtomicInteger might do the trick.

it
.map { it.id }
.map { it.substringAfter("emulator-") }
.map { it.toInt() }
.max()!! + 2
.let { it + usedPorts }
.let { (it.max() ?: 5552) + 2 }
.also { usedPorts.add(it) }
}
}
.map { it to it + 1 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class EmulatorsSpec : Spek({
connectedAdbDevices = connectedAdbDevices,
createAvd = createAvd,
applyConfig = applyConfig,
emulator = emulator,
emulatorCmd = emulator,
startEmulatorProcess = startEmulatorsProcess,
waitForEmulatorToStart = waitForEmulatorToStart,
findAvailablePortsForNewEmulator = findAvailablePortsForNewEmulator,
Expand Down