perf(devices): probe platform inventories concurrently - #1524
Merged
Conversation
An inventory lookup with no platform filter awaited each platform's toolchain in turn, so it cost their sum. Opening a physical iPhone by name spent 6.7s in resolve_target_device on a host with the Apple, Android and Vega toolchains installed, most of it enumerating platforms the request could not target (vega device list alone was 2.8s of it). The probes are independent, so run them concurrently and concatenate in selector order — the Linux local device still lands last, where it must be so it does not displace connected Android/Apple devices in implicit selection. Alternating A/B on one host against a cabled iPhone, three runs each: sequential 4065/4015/3912ms, concurrent 2359/2365/2295ms. A platform answering with a non-array still contributes nothing: spreading it used to throw into the per-platform catch, and that is now explicit.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while scoping #1522. The usbmux discovery idea was justified by
resolve_target_devicecosting 6672 ms in the #1517 cabled capture — but breaking that number down showed usbmux was aiming at the wrong target.Where the time actually went
Per-probe durations from that capture:
vega device listxctrace list devicesdevicectl list devicessimctl list devicesemulator -list-avdsadb devices -lThey ran essentially back to back, because
listLocalDeviceInventoryawaited each platform inside aforloop. An unfiltered lookup therefore cost the sum of every installed toolchain — and for anopentargeting an iPhone by name, most of that was spent enumerating platforms the request could never resolve to. The single largest contributor was Vega, which has nothing to do with the request.usbmuxd discovery would only have shaved the two Apple probes. Removing the serialization is worth more, and it helps Android and Vega users equally.
Change
Probe the platforms concurrently and concatenate the results in selector order. Ordering is load-bearing and preserved: the Linux local device must stay last so it does not displace connected Android/Apple devices in implicit auto-selection, and
Promise.allkeeps input order.Measured
Alternating A/B on one host against the same cabled iPhone, rebuilding between arms, three runs each:
~1.7x, about 1.65 s off every cold device resolution. (Absolute numbers are below the original 6.7 s because that capture was taken on a busier host; the ratio is what alternating runs control for.)
Behaviour preserved
catch;.flat()would instead have kept anundefinedin the list. A pre-existing test caught this, and it is now explicit.Testing
android, apple, vega, linuxin the result.pnpm check:affected --runpasses.