New issue
Advanced search Search tips

Issue 901709 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocked on:
issue 857548
issue 894363

Blocking:
issue 903751



Sign in to add a comment

Enable /Zc:dllexportInlines- for faster build on windows component build

Project Member Reported by tikuta@chromium.org, Nov 5

Issue description

I implemented the feature similar to fvisibility-inlines-hidden in clang-cl.
https://reviews.llvm.org/D51340

To enable this, we need to specify dllexport for some inline function and remove some inline specifier.

This is a tracking bug of the effort to enable /Zc:DllexportInlines-.
 
Components: Build
Labels: OS-Windows
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 5

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d4219df603e902eb0ae7f1fd02d55d16522eb15b

commit d4219df603e902eb0ae7f1fd02d55d16522eb15b
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Mon Nov 05 08:56:26 2018

Remove V8_INLINE from non-inlineable function from parser.h

I will enable /Zc:DllexportInlines- flags for faster build time on windows.
But the flag makes clang's -Wundefined-inline check more strict as a secondary effect.

Actually, having inline function specifier for the function not defined in header file seems bit strange.
Let me remove inline specifier from such functions.

Bug: chromium:857548,  chromium:901709 
Change-Id: Ic06d10e2445cfedc7af67b72154f93a51ac26853
Reviewed-on: https://chromium-review.googlesource.com/c/1186017
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57229}
[modify] https://crrev.com/d4219df603e902eb0ae7f1fd02d55d16522eb15b/src/parsing/parser.h

Blockedon: 857548
Summary: Enable /Zc:dllexportInlines- for faster build on windows component build (was: Enable /Zc:DllexportInlines- for faster build on windows component build)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e21f93382b27f96430d358579e80688c9de7df7b

commit e21f93382b27f96430d358579e80688c9de7df7b
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 05:29:28 2018

Add EXPORT specifier in /third_party/blink/renderer/core/layout/ng

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=atotic@chromium.org

Bug:  901709 
Change-Id: I0facb6e5743e74f65730a2a24179d5e83c71715f
Reviewed-on: https://chromium-review.googlesource.com/c/1322330
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605969}
[modify] https://crrev.com/e21f93382b27f96430d358579e80688c9de7df7b/third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.h

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e150bdfe3637e8afe0724544a9fe4169bff94a3

commit 9e150bdfe3637e8afe0724544a9fe4169bff94a3
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 05:52:05 2018

Add EXPORT specifier in /third_party/blink/renderer/core/dom

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=tkent@chromium.org

Bug:  901709 
Change-Id: I21a5a8b9a53058ec6b777af846757c6b6377805a
Reviewed-on: https://chromium-review.googlesource.com/c/1322253
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605973}
[modify] https://crrev.com/9e150bdfe3637e8afe0724544a9fe4169bff94a3/third_party/blink/renderer/core/dom/qualified_name.h
[modify] https://crrev.com/9e150bdfe3637e8afe0724544a9fe4169bff94a3/third_party/blink/renderer/core/dom/space_split_string.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0077193b2f5171e65e7480368d69c672c4e06ae8

commit 0077193b2f5171e65e7480368d69c672c4e06ae8
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 07:24:26 2018

Add EXPORT specifier in /third_party/blink/renderer/core/css

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=futhark@chromium.org

Bug:  901709 
Change-Id: I5adf66b516a0d07c9f58e195a1d0a2b7cb3e95ee
Reviewed-on: https://chromium-review.googlesource.com/c/1322176
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605983}
[modify] https://crrev.com/0077193b2f5171e65e7480368d69c672c4e06ae8/third_party/blink/renderer/core/css/css_initial_value.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/495c13bc20b3da30532f2d17a59542087c7b5cc4

commit 495c13bc20b3da30532f2d17a59542087c7b5cc4
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 14:03:18 2018

Add EXPORT specifier in /third_party/blink/renderer/platform/graphics

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=schenney@chromium.org

Bug:  901709 
Change-Id: I605f83511671f07dbf464fa1fd151954d6068da1
Reviewed-on: https://chromium-review.googlesource.com/c/1322332
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606034}
[modify] https://crrev.com/495c13bc20b3da30532f2d17a59542087c7b5cc4/third_party/blink/renderer/platform/graphics/gpu/xr_webgl_drawing_buffer.h

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/883557ad45a155bfe6b228e33c86846e10623255

commit 883557ad45a155bfe6b228e33c86846e10623255
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 14:27:47 2018

Add EXPORT specifier in /third_party/blink/renderer/core/paint

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=fmalita@chromium.org

Bug:  901709 
Change-Id: I32a755124e41532ceebc67900047ccb4580de9c2
Reviewed-on: https://chromium-review.googlesource.com/c/1322331
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606040}
[modify] https://crrev.com/883557ad45a155bfe6b228e33c86846e10623255/third_party/blink/renderer/core/paint/fragment_data.h
[modify] https://crrev.com/883557ad45a155bfe6b228e33c86846e10623255/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/510ff066cc06cb379f3678cc6b37e88a9294fc2b

commit 510ff066cc06cb379f3678cc6b37e88a9294fc2b
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 15:39:29 2018

Add EXPORT specifier in /gpu

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=ericrk@chromium.org

Bug:  901709 
Change-Id: Id41759fbb8b2234c24dbaea33fcc3eff2a97b593
Reviewed-on: https://chromium-review.googlesource.com/c/1322254
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606052}
[modify] https://crrev.com/510ff066cc06cb379f3678cc6b37e88a9294fc2b/gpu/ipc/client/gpu_channel_host.h

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a435089978e6dd68e94fe1a42eacc1a60270e02

commit 7a435089978e6dd68e94fe1a42eacc1a60270e02
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed Nov 07 18:36:55 2018

Add EXPORT specifier in /third_party/blink/renderer/platform/wtf

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=erik.corry@gmail.com

Bug:  901709 
Change-Id: I942c126cf68df4b242c7e49f05c2d7696e9d1b9e
Reviewed-on: https://chromium-review.googlesource.com/c/1322175
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606090}
[modify] https://crrev.com/7a435089978e6dd68e94fe1a42eacc1a60270e02/third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_contents.h

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c91afb7bb2f4d3f1e0edb63ee7b289edd77abac4

commit c91afb7bb2f4d3f1e0edb63ee7b289edd77abac4
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Thu Nov 08 00:00:07 2018

Add EXPORT specifier in /third_party/blink/renderer/core/style

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=ericwilligers@chromium.org

Bug:  901709 
Change-Id: Ib1b1c7a0649ce67620a9b25ec77d4c6ace44b546
Reviewed-on: https://chromium-review.googlesource.com/c/1322252
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606228}
[modify] https://crrev.com/c91afb7bb2f4d3f1e0edb63ee7b289edd77abac4/third_party/blink/renderer/core/style/shadow_data.h

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fef50b883ddc88a684b8f1eb4106097e1889078

commit 3fef50b883ddc88a684b8f1eb4106097e1889078
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Thu Nov 08 16:16:50 2018

Add EXPORT specifier in /net

To speed up build time on windows component build, I will enable newly added /Zc:dllexportInlines- flag. The flag is similar to -fvisibility-inlines-hidden in gcc/clang.

This flag makes inline member function is not exported if it does not have static local variables. But in few classes of chromium, we need to add additional EXPORT specifier to prevent link failure.

This is a part of effort adding EXPORT speicifer to enable the flag.

Master CL is
https://chromium-review.googlesource.com/c/chromium/src/+/1317069

This CL was uploaded by git cl split.

R=morlovich@chromium.org

Bug:  901709 
Change-Id: I1ef0284430ca511ab15dfe53fcf49bfe758a03f7
Reviewed-on: https://chromium-review.googlesource.com/c/1322333
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606488}
[modify] https://crrev.com/3fef50b883ddc88a684b8f1eb4106097e1889078/net/socket/client_socket_pool_base.h

Blocking: 903751
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b06ee43ef9c94b4cc4b4661136cb14d00d8b4e36

commit b06ee43ef9c94b4cc4b4661136cb14d00d8b4e36
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Mon Nov 12 03:37:07 2018

Enable /Zc:dllexportInlines-

This CL breaks ABI compatibility with cl.exe (which we no longer need).

This patch enables /Zc:dllexportInlines- flag on windows component build for faster build time.
When we enable this flag, inline member functions are not exported by default if the function does not have static local variable.
By not exporting inline function, clang does not emit code and symbols for inline functions.
This improves compilation time.

This flag also reduces the size of obj size in windows component build. Smaller obj and reduced number of symbols improve link time largely.
Additionally, for builds using distributed build system like goma, lower network usage by smaller obj files makes build faster.

This is full build time comparison of 'chrome' target with -j960 on 24C/48T Win10 machine.

|----------------------------+-----------------------+--------|
| flag /Zc:dllexportInlines- | on                    | off    |
|----------------------------+-----------------------+--------|
| warm goma backend cache    |  9m13s (1.31x faster) | 12m3s  |
| cold goma backend cache    | 11m57s (1.40x faster) | 16m45s |
|----------------------------+-----------------------+--------|

Detailed goma build stats comparison w/wo this flag is shown in
https://docs.google.com/document/d/1_W-RsPe5LMz8LOm2Cz1hQ9OlmpTNARbBzARaM81RBYM/edit?usp=sharing


Concept of the flag is similar to -fvisibility-inlines-hidden in gcc/clang.

Bug:  901709 
Change-Id: I943a47b2098775f2474c5a8a05b8a6a59628044e
Reviewed-on: https://chromium-review.googlesource.com/c/1317069
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607137}
[modify] https://crrev.com/b06ee43ef9c94b4cc4b4661136cb14d00d8b4e36/build/config/win/BUILD.gn

Status: Fixed (was: Untriaged)
The flag is enabled.

Sign in to add a comment