Windows bulding: Make dependency generation not quite as talkative #15006

levitte · 2021-04-23T14:24:00Z

The modified way to generate .d files had an unfortunate side effect,
that it outputs the whole preprocessed file and not just the dependency
lines, at least with MSVC's cl. That gave util/add-depends.pl a whole
lot more to read through, which impacts greatly on the performance of
dependency treatment.

We modify the process by adding a config target attribute 'make_depend',
which can be any suitable command for generating such lines. All it
needs is to also accept C flags and macro definitions.

Fixes #14994

The modified way to generate .d files had an unfortunate side effect, that it outputs the whole preprocessed file and not just the dependency lines, at least with MSVC's cl. That gave util/add-depends.pl a whole lot more to read through, which impacts greatly on the performance of dependency treatment. We modify the process by adding a config target attribute 'make_depend', which can be any suitable command for generating such lines. All it needs is to also accept C flags and macro definitions. Fixes openssl#14994

levitte · 2021-04-23T14:24:50Z

This is an alternative to #15001

Configurations/50-cppbuilder.conf

…tive

levitte · 2021-04-23T16:23:47Z

@tanzislam, it would be great if you would test this.

tanzislam

This is good. The "master"-branch is apparently broken somehow with C++Builder (guessing due to 6b29784):

> perl Configure BC-32 --prefix=%CD% no-asm
...
> make -N
...
        cmd /C ""ilink32" -x -Gn -q -w-dup -j"C:\Program Files (x86)\Embarcadero\Studio\20.0\lib\win32c\release" -L"C:\Program Files (x86)\Embarcadero\Studio\20.0\lib\win32c\release" -aa -Tpd c0d32.obj  -x -Gn -q -w-dup -j"C:\Program Files (x86)\Embarcadero\Studio\20.0\lib\win32c\release" -L"C:\Program Files (x86)\Embarcadero\Studio\20.0\lib\win32c\release" @MAKE0001.@@@  || (DEL /Q libcrypto-3.* libcrypto.lib & EXIT 1)"
Error: Unresolved external 'WspiapiLoad' referenced from C:\USERS\T_17_\SOURCE\REPOS\CRYPTOPALS\OPENSSL_ANOTHER\CRYPTO\BIO\LIBCRYPTO-SHLIB-B_ADDR.OBJ
Error: Unable to perform link

I'll investigate and fix that separately. Don't want to delay fixing the MSVC-build.

UPDATE: Raised https://quality.embarcadero.com/browse/RSP-33806. Can't think of a workaround, so raised #15025 to undo that commit for C++Builder.

mspncp · 2021-04-26T07:22:16Z

Raised https://quality.embarcadero.com/browse/RSP-33806.

@tanzislam is there a guest login for this link?

Previously, we had dependency making pretty much hard coded in the build file templates, with a bit of an exception for Unix family platforms, where we had different cases depending on what dependency making program was found. With the Embarcadero C++ builder, a separate scheme appeared, with a different logic. This change merges the two, and introduces two config target attributes: makedepcmd The program to use, where this is relevant. This replaces the earlier configuration attribute 'makedepprog'. makedep_scheme This is a keyword that can be used by build files templates to produce different sorts of commands, but most importantly, to pass as argument to util/add-depend.pl, which uses this keyword as a "producer" for the dependency lines. If the config target doesn't define the 'makedep_scheme' attribute, Configure tries to figure it out by looking for GCC compatible compilers or for the 'makedepend' command.

tanzislam · 2021-04-26T07:29:35Z

Unfortunately not. It's free to create an Embarcadero account at https://www.embarcadero.com/products/cbuilder/starter/free-download

I gave them a minimal example that does not link when compiled as C, but does so as C++:

__inline void func(void) { return; }

int main(void)
{
    func();
    return 0;
}

It was discovered that MSVC has localized /showIncludes output. Fortunately, it still seems to follow the same generic format, so we can adapt the regular expression to make it language agnostic. Fixes openssl#14994

levitte · 2021-04-26T07:33:03Z

I did a bit or rework to make the whole dependency making stuff less of a hack.
I also changed util/add-depends.pl to handle localized MSVC output

mspncp · 2021-04-26T07:50:09Z

Restating my question #14994 (comment) here, where it seems more fitting:

@levitte are you saying that the 'makedepend' configure option is not necessary for a full build, that it's just an optimization for further builds?

If that is the case, then the 'makedepend' option should be disabled by default. Because regular users will build&install the binaries in one go using configure, make, make install. So there is no benefit from creating the dependency files, it only slows everything down. Only openssl developers working on the source code would benefit from this optimization. (And the benefit is limited by the fact that switching between topic branches very often requires reconfiguration and a full rebuild.)

My main motivation is that we ought to get the terrible build times on Windows back to reasonable values before the beta release (issue #14055). Using no-makedepend and no-fips I was able to reduce the build times significantly:
BUILD OPENSSL
20:39:39,53 start configure
20:40:21,36 start nmake
20:53:34,86 start nmake install
20:57:17,08 start nmake clean
20:57:50,51 done
A more detailed report will follow soon on #14055.

levitte · 2021-04-26T07:56:10Z

@mspncp, I would rather have the question of makedepend being enable or disabled by default to be a separate issue. Please don't pile more stuff into one issue.

mspncp · 2021-04-26T07:57:01Z

Ok, no problem.

Configurations/descrip.mms.tmpl

util/add-depends.pl

levitte · 2021-04-27T09:34:11Z

@openssl, a review, please?

openssl-machine · 2021-04-28T11:00:16Z

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

mspncp · 2021-04-28T18:02:20Z

The only failure was that the AppVeyor build was cancelled.

The modified way to generate .d files had an unfortunate side effect, that it outputs the whole preprocessed file and not just the dependency lines, at least with MSVC's cl. That gave util/add-depends.pl a whole lot more to read through, which impacts greatly on the performance of dependency treatment. We modify the process by adding a config target attribute 'make_depend', which can be any suitable command for generating such lines. All it needs is to also accept C flags and macro definitions. Fixes #14994 Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matthias St. Pierre <[email protected]> (Merged from #15006)

Previously, we had dependency making pretty much hard coded in the build file templates, with a bit of an exception for Unix family platforms, where we had different cases depending on what dependency making program was found. With the Embarcadero C++ builder, a separate scheme appeared, with a different logic. This change merges the two, and introduces two config target attributes: makedepcmd The program to use, where this is relevant. This replaces the earlier configuration attribute 'makedepprog'. makedep_scheme This is a keyword that can be used by build files templates to produce different sorts of commands, but most importantly, to pass as argument to util/add-depend.pl, which uses this keyword as a "producer" for the dependency lines. If the config target doesn't define the 'makedep_scheme' attribute, Configure tries to figure it out by looking for GCC compatible compilers or for the 'makedepend' command. Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matthias St. Pierre <[email protected]> (Merged from #15006)

It was discovered that MSVC has localized /showIncludes output. Fortunately, it still seems to follow the same generic format, so we can adapt the regular expression to make it language agnostic. Fixes #14994 Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matthias St. Pierre <[email protected]> (Merged from #15006)

levitte · 2021-04-28T19:36:47Z

Merged

0bd138b Windows bulding: Make dependency generation not quite as talkative
2e535eb Configuration: rework how dependency making is handled
3babc1e util/add-depends.pl: Adapt to localized /showIncludes output

levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Apr 23, 2021

levitte mentioned this pull request Apr 23, 2021

Avoid emitting preprocessed output for dependency info #15001

Closed

2 tasks

levitte commented Apr 23, 2021

View reviewed changes

Configurations/50-cppbuilder.conf Outdated Show resolved Hide resolved

fixup! Windows bulding: Make dependency generation not quite as talka…

6d3d85a

…tive

tanzislam approved these changes Apr 23, 2021

View reviewed changes

mspncp self-requested a review April 23, 2021 17:29

levitte mentioned this pull request Apr 26, 2021

nmake install stuck in first step in windows #14994

Closed

util/add-depends.pl: Adapt to localized /showIncludes output

032dff6

It was discovered that MSVC has localized /showIncludes output. Fortunately, it still seems to follow the same generic format, so we can adapt the regular expression to make it language agnostic. Fixes openssl#14994

t8m reviewed Apr 26, 2021

View reviewed changes

Configurations/descrip.mms.tmpl Outdated Show resolved Hide resolved

fixup! Configuration: rework how dependency making is handled

6b1fb6c

tanzislam reviewed Apr 27, 2021

View reviewed changes

util/add-depends.pl Outdated Show resolved Hide resolved

fixup! util/add-depends.pl: Adapt to localized /showIncludes output

0424bea

levitte force-pushed the fix-14994 branch from 8b649c4 to 0424bea Compare April 27, 2021 09:32

t8m approved these changes Apr 27, 2021

View reviewed changes

t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 27, 2021

mspncp approved these changes Apr 27, 2021

View reviewed changes

mspncp added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Apr 28, 2021

levitte closed this Apr 28, 2021

levitte deleted the fix-14994 branch April 28, 2021 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows bulding: Make dependency generation not quite as talkative #15006

Windows bulding: Make dependency generation not quite as talkative #15006

levitte commented Apr 23, 2021

levitte commented Apr 23, 2021

levitte commented Apr 23, 2021

tanzislam left a comment •

edited

Loading

mspncp commented Apr 26, 2021

tanzislam commented Apr 26, 2021 •

edited

Loading

levitte commented Apr 26, 2021

mspncp commented Apr 26, 2021

levitte commented Apr 26, 2021

mspncp commented Apr 26, 2021

levitte commented Apr 27, 2021

openssl-machine commented Apr 28, 2021

mspncp commented Apr 28, 2021

levitte commented Apr 28, 2021

Windows bulding: Make dependency generation not quite as talkative #15006

Windows bulding: Make dependency generation not quite as talkative #15006

Conversation

levitte commented Apr 23, 2021

levitte commented Apr 23, 2021

levitte commented Apr 23, 2021

tanzislam left a comment • edited Loading

Choose a reason for hiding this comment

mspncp commented Apr 26, 2021

tanzislam commented Apr 26, 2021 • edited Loading

levitte commented Apr 26, 2021

mspncp commented Apr 26, 2021

levitte commented Apr 26, 2021

mspncp commented Apr 26, 2021

levitte commented Apr 27, 2021

openssl-machine commented Apr 28, 2021

mspncp commented Apr 28, 2021

levitte commented Apr 28, 2021

tanzislam left a comment •

edited

Loading

tanzislam commented Apr 26, 2021 •

edited

Loading