Bug 223727

Summary: run-minibrowser is slow
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: Tools / TestsAssignee: Cameron McCormack (:heycam) <heycam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, jbedard, sam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch none

Description Cameron McCormack (:heycam) 2021-03-24 20:27:55 PDT
On my machine, run-minibrowser takes 2s to do its preparatory work before the MiniBrowser binary is launched.  (I measured this just with `time run-minibrowser --help`.)
Comment 1 Cameron McCormack (:heycam) 2021-03-24 20:35:26 PDT
Created attachment 424213 [details]
Patch
Comment 2 Cameron McCormack (:heycam) 2021-03-24 20:37:49 PDT
Created attachment 424214 [details]
Patch
Comment 3 Cameron McCormack (:heycam) 2021-03-24 20:43:47 PDT
Comment on attachment 424214 [details]
Patch

Something went wrong for non macOS builds.
Comment 4 Cameron McCormack (:heycam) 2021-03-24 21:49:22 PDT
Created attachment 424217 [details]
Patch
Comment 5 Sam Weinig 2021-03-25 13:09:47 PDT
Comment on attachment 424217 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424217&action=review

> Tools/ChangeLog:19
> +        run-minibrowser needs to know what port it's running on, since that
> +        affects the build directory to look in to find the MiniBrowser binary.
> +        On macOS, webkitdirs.pm's determinePortName ends up running
> +        `xcodebuild -showsdks` to see if the current SDK has an internal
> +        variant available, but this is slow.  But we don't need to know the
> +        exact SDK name here, just the SDK platform name, to determine the
> +        port name.
> +
> +        So we shuffle some code around to avoid calling `xcodebuild -showsdks`
> +        where we can.  This reduces the time spent in run-minibrowser before
> +        MiniBrowser is launched (crudely measured with `time run-minibrowser
> +        --help`) from 2s to 0.6s on this machine.

Very nice!

Can we avoid ever calling `xcodebuild -showsdks`, and just call `xcrun --show-sdk-path` for current SDK determination instead? That should be really fast.
Comment 6 Sam Weinig 2021-03-25 13:13:41 PDT
(In reply to Sam Weinig from comment #5)
> Comment on attachment 424217 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424217&action=review
> 
> > Tools/ChangeLog:19
> > +        run-minibrowser needs to know what port it's running on, since that
> > +        affects the build directory to look in to find the MiniBrowser binary.
> > +        On macOS, webkitdirs.pm's determinePortName ends up running
> > +        `xcodebuild -showsdks` to see if the current SDK has an internal
> > +        variant available, but this is slow.  But we don't need to know the
> > +        exact SDK name here, just the SDK platform name, to determine the
> > +        port name.
> > +
> > +        So we shuffle some code around to avoid calling `xcodebuild -showsdks`
> > +        where we can.  This reduces the time spent in run-minibrowser before
> > +        MiniBrowser is launched (crudely measured with `time run-minibrowser
> > +        --help`) from 2s to 0.6s on this machine.
> 
> Very nice!
> 
> Can we avoid ever calling `xcodebuild -showsdks`, and just call `xcrun
> --show-sdk-path` for current SDK determination instead? That should be
> really fast.

Actually, ignore this, I see it want's to figure out if an internal SDK installed, and that won't do it on its own.
Comment 7 Cameron McCormack (:heycam) 2021-03-25 13:57:43 PDT
I have a feeling we can do our own grovelling around the currently selected Xcode's directories to find the SDKs ourselves.  That at least contains all the SDKs on my system that show up in `xcodebuild -showsdks`.  But I'm not confident there aren't other locations that might be searched too.
Comment 8 EWS 2021-03-25 23:57:41 PDT
Committed r275081: <https://commits.webkit.org/r275081>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424217 [details].
Comment 9 Radar WebKit Bug Importer 2021-03-25 23:58:48 PDT
<rdar://problem/75874999>
Comment 10 Jonathan Bedard 2021-03-26 11:11:51 PDT
Reverted r275081 for reason:

Broke Apple Internal builds

Committed r275105 (235813@main): <https://commits.webkit.org/235813@main>
Comment 11 Cameron McCormack (:heycam) 2021-03-29 16:27:21 PDT
Created attachment 424600 [details]
Patch
Comment 12 EWS 2021-03-29 19:42:42 PDT
Committed r275196: <https://commits.webkit.org/r275196>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424600 [details].