-
Notifications
You must be signed in to change notification settings - Fork 17
Speed up emulator port allocation #46
base: master
Are you sure you want to change the base?
Speed up emulator port allocation #46
Conversation
Storing a list of used ports so we don't need to wait for the emulator start command to finish before starting the next emulator.
@@ -219,17 +211,22 @@ private fun emulatorBinary(args: Commands.Start): String = | |||
emulator | |||
} | |||
|
|||
private val usedPorts: MutableList<Int> = mutableListOf() |
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.
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
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.
Ok I might be wrong here, if this is just the used ports it should be fine
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.
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.
What if we go the other way around? We can find all ports for required amount of emulators upfront (yes there is a race possible until emulator actually allocates the port, but it's a bit more safe I guess) |
if (it.isEmpty()) { | ||
5554 | ||
} else { | ||
synchronized(usedPorts) { |
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.
I'd use ConcurrentHashMap().toSet()
so you have thread-safe MutableSet
without need for synchronization
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.
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.
Add test for port allocation
Store a list of used ports so we don't need to wait for the emulator start command to finish before starting the next emulator.