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

Conversation

jakob-grabner
Copy link

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.

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()
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.

@artem-zinnatullin
Copy link
Contributor

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) {
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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants