Bug 182446 - [Win] Enable multi cpu building on MSBuild.
Summary: [Win] Enable multi cpu building on MSBuild.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-02 11:57 PST by Basuke Suzuki
Modified: 2018-02-05 11:19 PST (History)
10 users (show)

See Also:


Attachments
patch (1.54 KB, patch)
2018-02-02 12:03 PST, Basuke Suzuki
no flags Details | Formatted Diff | Diff
patch (1.88 KB, patch)
2018-02-02 12:20 PST, Basuke Suzuki
no flags Details | Formatted Diff | Diff
PATCH (1.88 KB, patch)
2018-02-03 20:07 PST, Basuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 2018-02-02 11:57:58 PST
Compiling itself already uses multi cores, but other stuff is still on single core. This patch enables that by passing an argument to MSBuild.exe.
Comment 1 Basuke Suzuki 2018-02-02 12:03:15 PST
Created attachment 332990 [details]
patch
Comment 2 Basuke Suzuki 2018-02-02 12:15:11 PST
32 min => 21 min !

(Core i7-8700K 3.7GHz 6 core)
Comment 3 Basuke Suzuki 2018-02-02 12:20:26 PST
Created attachment 332992 [details]
patch
Comment 4 Alex Christensen 2018-02-02 16:45:52 PST
Comment on attachment 332992 [details]
patch

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

> Tools/Scripts/webkitdirs.pm:438
> +        $numberOfCPUs = 1;

Don't we want an implementation for non-cygwin Windows?
Comment 5 Daniel Bates 2018-02-02 19:47:33 PST
Comment on attachment 332992 [details]
patch

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

> Tools/Scripts/webkitdirs.pm:1922
> +    my $maxCpuCount = '/maxcpucount:' . numberOfCPUs();

Nit: maxCpuCount  => maxCPUCount

(Since CPU is an abbreviation)
Comment 6 Don Olmstead 2018-02-02 20:54:22 PST
(In reply to Alex Christensen from comment #4)
> Comment on attachment 332992 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332992&action=review
> 
> > Tools/Scripts/webkitdirs.pm:438
> > +        $numberOfCPUs = 1;
> 
> Don't we want an implementation for non-cygwin Windows?

I THINK the environment variable checked above should be set on windows. There looks to be a perl module that will tell you the number of CPUs as well http://search.cpan.org/~burak/Sys-Info-Base-0.7802/lib/Sys/Info/Device/CPU.pm which sounds like it calls into WMI
Comment 7 Basuke Suzuki 2018-02-03 20:07:10 PST
> Don't we want an implementation for non-cygwin Windows?

Don is right. Here's the source. https://msdn.microsoft.com/en-us/library/fd7hxfdd(v=vs.84).aspx

> Nit: maxCpuCount  => maxCPUCount
> (Since CPU is an abbreviation)

Right. I'll send another patch.
Comment 8 Basuke Suzuki 2018-02-03 20:07:46 PST
Created attachment 333044 [details]
PATCH
Comment 9 WebKit Commit Bot 2018-02-05 11:15:14 PST
Comment on attachment 333044 [details]
PATCH

Clearing flags on attachment: 333044

Committed r228106: <https://trac.webkit.org/changeset/228106>
Comment 10 WebKit Commit Bot 2018-02-05 11:15:15 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2018-02-05 11:19:54 PST
<rdar://problem/37244015>