Why are we re-linking?


(Dave Abrahams) #1

Can anyone explain why we're re-linking all these C++ executables when I
make changes only to the standard library?

[5/130] Linking CXX static library lib/libswiftBasic.a
[6/130] Linking CXX shared library lib/libswiftDemangle.dylib
[7/130] Linking CXX executable bin/swift
[8/130] Linking CXX executable bin/swift-demangle
[9/130] Linking CXX executable bin/sil-opt
[10/130] Linking CXX executable bin/swift-ide-test
[11/130] Linking CXX executable bin/lldb-moduleimport-test
[12/130] Linking CXX executable bin/sil-extract
[13/130] Linking CXX executable bin/swift-llvm-opt
[14/130] Linking CXX shared library lib/libsourcekitdInProc.dylib
[15/130] Linking CXX executable lib/sourcekitd.framework/Versions/A/XPCServices/SourceKitService.xpc/Contents/MacOS/SourceKitService
[16/130] Compiling /Users/dave/src/s/build/Ninja-RelWithDebInfoAssert+stdlib-DebugAssert/swift-macosx-x86_64/stdlib/public/core/macosx/x86_64/Swift.o
[17/130] Linking CXX shared library lib/sourcekitd.framework/Versions/A/sourcekitd
[18/130] Linking CXX executable bin/sourcekitd-test
[19/130] Linking CXX executable bin/sourcekitd-repl
[20/130] Linking CXX executable bin/complete-test

I imagine we might speed up stdlib development cycles a bit if we could
avoid that work.

Thanks,

···

--
Dave


(Jordan Rose) #2

I imagine it's because your git hash has changed, which is used in the --version output for swiftc. I'm not sure how to avoid that cost entirely, but we could add a CMake option to not do it (which you could set locally), and we could probably move it to a library that isn't used by most of those tools (so that we're only re-linking swiftc).

Jordan

···

On Apr 6, 2016, at 10:48, Dave Abrahams via swift-dev <swift-dev@swift.org> wrote:

Can anyone explain why we're re-linking all these C++ executables when I
make changes only to the standard library?

[5/130] Linking CXX static library lib/libswiftBasic.a
[6/130] Linking CXX shared library lib/libswiftDemangle.dylib
[7/130] Linking CXX executable bin/swift
[8/130] Linking CXX executable bin/swift-demangle
[9/130] Linking CXX executable bin/sil-opt
[10/130] Linking CXX executable bin/swift-ide-test
[11/130] Linking CXX executable bin/lldb-moduleimport-test
[12/130] Linking CXX executable bin/sil-extract
[13/130] Linking CXX executable bin/swift-llvm-opt
[14/130] Linking CXX shared library lib/libsourcekitdInProc.dylib
[15/130] Linking CXX executable lib/sourcekitd.framework/Versions/A/XPCServices/SourceKitService.xpc/Contents/MacOS/SourceKitService
[16/130] Compiling /Users/dave/src/s/build/Ninja-RelWithDebInfoAssert+stdlib-DebugAssert/swift-macosx-x86_64/stdlib/public/core/macosx/x86_64/Swift.o
[17/130] Linking CXX shared library lib/sourcekitd.framework/Versions/A/sourcekitd
[18/130] Linking CXX executable bin/sourcekitd-test
[19/130] Linking CXX executable bin/sourcekitd-repl
[20/130] Linking CXX executable bin/complete-test

I imagine we might speed up stdlib development cycles a bit if we could
avoid that work.

Thanks,

--
Dave

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Chris Lattner) #3

Could we move the hash to be a text file stored next to the executables in the installdir? That way the text file gets updated, but not the binaries?

-Chris

···

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

I imagine it's because your git hash has changed, which is used in the --version output for swiftc. I'm not sure how to avoid that cost entirely, but we could add a CMake option to not do it (which you could set locally), and we could probably move it to a library that isn't used by most of those tools (so that we're only re-linking swiftc).


(Chris Bieneman) #4

In Clang this is handled by having a CMake option control whether or not the SCM revision is embedded in the binary (CLANG_APPEND_VC_REV). Clang defaults the option to 'Off' so that engineering workflows don't need to override it, but it is frequently enabled on bots and in distributions.

The build system and code in the compiler to do this is very simple, and could likely be applied to Swift.

The full CMake code from <clang>/CMakeLists.txt is:

option(CLANG_APPEND_VC_REV
  "Append the version control system revision id to clang version spew" OFF)
if(CLANG_APPEND_VC_REV)
  if(NOT SVN_REVISION)
    # This macro will set SVN_REVISION in the parent scope
    add_version_info_from_vcs(VERSION_VAR)
  endif()

  if(SVN_REVISION)
    add_definitions(-DSVN_REVISION="${SVN_REVISION}")
  endif()
endif()

-Chris

···

On Apr 7, 2016, at 2:01 PM, Chris Lattner via swift-dev <swift-dev@swift.org> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

I imagine it's because your git hash has changed, which is used in the --version output for swiftc. I'm not sure how to avoid that cost entirely, but we could add a CMake option to not do it (which you could set locally), and we could probably move it to a library that isn't used by most of those tools (so that we're only re-linking swiftc).

Could we move the hash to be a text file stored next to the executables in the installdir? That way the text file gets updated, but not the binaries?

-Chris
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Jordan Rose) #5

Oh, that's clever. Maybe that's good enough for local builds. I'd want to be careful about it for the binary we ship (to not waste an fstat).

FWIW this isn't quite just --version output; we also stamp it into swiftmodule files. But any build with an actual submission tag will use that instead.

Jordan

···

On Apr 7, 2016, at 14:01, Chris Lattner <clattner@apple.com> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

I imagine it's because your git hash has changed, which is used in the --version output for swiftc. I'm not sure how to avoid that cost entirely, but we could add a CMake option to not do it (which you could set locally), and we could probably move it to a library that isn't used by most of those tools (so that we're only re-linking swiftc).

Could we move the hash to be a text file stored next to the executables in the installdir? That way the text file gets updated, but not the binaries?


(Dave Abrahams) #6

In Clang this is handled by having a CMake option control whether or
not the SCM revision is embedded in the binary
(CLANG_APPEND_VC_REV). Clang defaults the option to 'Off' so that
engineering workflows don't need to override it, but it is frequently
enabled on bots and in distributions.

The build system and code in the compiler to do this is very simple,
and could likely be applied to Swift.

Awesome; care to take a crack at it? :slight_smile:

···

on Fri Apr 08 2016, Chris Bieneman <cbieneman-AT-apple.com> wrote:

The full CMake code from <clang>/CMakeLists.txt is:

option(CLANG_APPEND_VC_REV
  "Append the version control system revision id to clang version spew" OFF)
if(CLANG_APPEND_VC_REV)
  if(NOT SVN_REVISION)
    # This macro will set SVN_REVISION in the parent scope
    add_version_info_from_vcs(VERSION_VAR)
  endif()

  if(SVN_REVISION)
    add_definitions(-DSVN_REVISION="${SVN_REVISION}")
  endif()
endif()

-Chris

On Apr 7, 2016, at 2:01 PM, Chris Lattner via swift-dev <swift-dev@swift.org> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

I imagine it's because your git hash has changed, which is used in
the --version output for swiftc. I'm not sure how to avoid that
cost entirely, but we could add a CMake option to not do it (which
you could set locally), and we could probably move it to a library
that isn't used by most of those tools (so that we're only
re-linking swiftc).

Could we move the hash to be a text file stored next to the
executables in the installdir? That way the text file gets updated,
but not the binaries?

-Chris
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

--
Dave


(Rainer Brockerhoff) #7

Re-reinventing resource forks again...? :slight_smile:

···

On 4/11/16 14:22, Jordan Rose via swift-dev wrote:

On Apr 7, 2016, at 14:01, Chris Lattner <clattner@apple.com >> <mailto:clattner@apple.com>> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev >>> <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

Could we move the hash to be a text file stored next to the
executables in the installdir? That way the text file gets updated,
but not the binaries?

Oh, that's clever. Maybe that's good enough for local builds. I'd want
to be careful about it for the binary we ship (to not waste an fstat).

--
Rainer Brockerhoff <rainer@brockerhoff.net>
Belo Horizonte, Brazil
"In the affairs of others even fools are wise
In their own business even sages err."
http://brockerhoff.net/blog/


#8

Hi all,

getSwiftFullVersion output must take account stdlib revsion?

How about get the revision hash from the Swift repository
*excluding* stdlib/, test/ and validation-test/ directories.

git log -1 --pretty=format:%H -- \
    . \
    :(exclude)stdlib \
    :(exclude)test \
    :(exclude)validation-test

Instead of,

git log -1 --pretty=format:%H

Or more specifically, just include only "compiler" related directories:

git log -1 --pretty=format:%H -- include/ lib/ tools/

I've wrote PoC code

···

2016-04-12 2:22 GMT+09:00 Jordan Rose via swift-dev <swift-dev@swift.org>:

On Apr 7, 2016, at 14:01, Chris Lattner <clattner@apple.com> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev < > swift-dev@swift.org> wrote:

I imagine it's because your git hash has changed, which is used in the
--version output for swiftc. I'm not sure how to avoid that cost entirely,
but we could add a CMake option to not do it (which you could set locally),
and we could probably move it to a library that isn't used by most of those
tools (so that we're only re-linking swiftc).

Could we move the hash to be a text file stored next to the executables in
the installdir? That way the text file gets updated, but not the binaries?

Oh, that's clever. Maybe that's good enough for local builds. I'd want to
be careful about it for the binary we ship (to not waste an fstat).

FWIW this isn't *quite* just --version output; we also stamp it into
swiftmodule files. But any build with an actual submission tag will use
that instead.

Jordan

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


#9

Sorry, accidentally clicked send button..

I've wrote PoC code here:
https://github.com/rintaro/swift/commit/9bf18d46f4933ace03948417087bade084104edb

Of course, this changes the semantics of `--version` output.
So I'm not sure we can accept this or not.
Any thought?

Rintaro

···

2016-05-06 13:08 GMT+09:00 rintaro ishizaki <fs.output@gmail.com>:

Hi all,

getSwiftFullVersion output must take account stdlib revsion?

How about get the revision hash from the Swift repository
*excluding* stdlib/, test/ and validation-test/ directories.

git log -1 --pretty=format:%H -- \
    . \
    :(exclude)stdlib \
    :(exclude)test \
    :(exclude)validation-test

Instead of,

git log -1 --pretty=format:%H

Or more specifically, just include only "compiler" related directories:

git log -1 --pretty=format:%H -- include/ lib/ tools/

I've wrote PoC code

2016-04-12 2:22 GMT+09:00 Jordan Rose via swift-dev <swift-dev@swift.org>:

On Apr 7, 2016, at 14:01, Chris Lattner <clattner@apple.com> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev < >> swift-dev@swift.org> wrote:

I imagine it's because your git hash has changed, which is used in the
--version output for swiftc. I'm not sure how to avoid that cost entirely,
but we could add a CMake option to not do it (which you could set locally),
and we could probably move it to a library that isn't used by most of those
tools (so that we're only re-linking swiftc).

Could we move the hash to be a text file stored next to the executables
in the installdir? That way the text file gets updated, but not the
binaries?

Oh, that's clever. Maybe that's good enough for local builds. I'd want to
be careful about it for the binary we ship (to not waste an fstat).

FWIW this isn't *quite* just --version output; we also stamp it into
swiftmodule files. But any build with an actual submission tag will use
that instead.

Jordan

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Jordan Rose) #10

Hm. That might be a nice balance, and it’s not entirely a lie: that’s the version of the compiler that you’re using, if not the stdlib and runtime. I’d still like to put it behind a flag, so that we can turn it off it certain configurations, like #2105 <https://github.com/apple/swift/pull/2105> attempted to do.

Dmitri, any comments, since you caught the issue last time?

Jordan

···

On May 5, 2016, at 21:12, rintaro ishizaki <fs.output@gmail.com> wrote:

Sorry, accidentally clicked send button..

I've wrote PoC code here:
https://github.com/rintaro/swift/commit/9bf18d46f4933ace03948417087bade084104edb

Of course, this changes the semantics of `--version` output.
So I'm not sure we can accept this or not.
Any thought?

Rintaro

2016-05-06 13:08 GMT+09:00 rintaro ishizaki <fs.output@gmail.com <mailto:fs.output@gmail.com>>:
Hi all,

getSwiftFullVersion output must take account stdlib revsion?

How about get the revision hash from the Swift repository
*excluding* stdlib/, test/ and validation-test/ directories.

git log -1 --pretty=format:%H -- \
    . \
    :(exclude)stdlib \
    :(exclude)test \
    :(exclude)validation-test

Instead of,

git log -1 --pretty=format:%H

Or more specifically, just include only "compiler" related directories:

git log -1 --pretty=format:%H -- include/ lib/ tools/

I've wrote PoC code

2016-04-12 2:22 GMT+09:00 Jordan Rose via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>>:

On Apr 7, 2016, at 14:01, Chris Lattner <clattner@apple.com <mailto:clattner@apple.com>> wrote:

On Apr 6, 2016, at 11:08 AM, Jordan Rose via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

I imagine it's because your git hash has changed, which is used in the --version output for swiftc. I'm not sure how to avoid that cost entirely, but we could add a CMake option to not do it (which you could set locally), and we could probably move it to a library that isn't used by most of those tools (so that we're only re-linking swiftc).

Could we move the hash to be a text file stored next to the executables in the installdir? That way the text file gets updated, but not the binaries?

Oh, that's clever. Maybe that's good enough for local builds. I'd want to be careful about it for the binary we ship (to not waste an fstat).

FWIW this isn't quite just --version output; we also stamp it into swiftmodule files. But any build with an actual submission tag will use that instead.

Jordan

_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev


(Dmitri Gribenko) #11

I think this would be a reasonable balance for local development. For
releases that CI produces I think this change will cause confusion
because the git hashes that are tagged would not match the git hashes
included in the compiler version.

Dmitri

···

On Fri, May 6, 2016 at 9:16 AM, Jordan Rose <jordan_rose@apple.com> wrote:

Hm. That might be a nice balance, and it’s not entirely a lie: that’s the
version of the compiler that you’re using, if not the stdlib and runtime.
I’d still like to put it behind a flag, so that we can turn it off it
certain configurations, like #2105 attempted to do.

Dmitri, any comments, since you caught the issue last time?

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Jordan Rose) #12

Oh, agreed. I think we wouldn’t have an issue there because none of those are incremental, but we could disable this for all buildbots if we wanted to be conservative.

Jordan

···

On May 6, 2016, at 09:23, Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Fri, May 6, 2016 at 9:16 AM, Jordan Rose <jordan_rose@apple.com> wrote:

Hm. That might be a nice balance, and it’s not entirely a lie: that’s the
version of the compiler that you’re using, if not the stdlib and runtime.
I’d still like to put it behind a flag, so that we can turn it off it
certain configurations, like #2105 attempted to do.

Dmitri, any comments, since you caught the issue last time?

I think this would be a reasonable balance for local development. For
releases that CI produces I think this change will cause confusion
because the git hashes that are tagged would not match the git hashes
included in the compiler version.


(Daniel Dunbar) #13

Is it so much work to move the data out of line (as Chris suggested) that we shouldn't just do that and involve another "mode"?

- Daniel

···

On May 6, 2016, at 9:23 AM, Dmitri Gribenko via swift-dev <swift-dev@swift.org> wrote:

On Fri, May 6, 2016 at 9:16 AM, Jordan Rose <jordan_rose@apple.com> wrote:

Hm. That might be a nice balance, and it’s not entirely a lie: that’s the
version of the compiler that you’re using, if not the stdlib and runtime.
I’d still like to put it behind a flag, so that we can turn it off it
certain configurations, like #2105 attempted to do.

Dmitri, any comments, since you caught the issue last time?

I think this would be a reasonable balance for local development. For
releases that CI produces I think this change will cause confusion
because the git hashes that are tagged would not match the git hashes
included in the compiler version.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


#14

I've updated the code to make that behavior optional and controllable:
https://github.com/rintaro/swift/commit/bfac557

Revision from only lib/, include/, and tools/:

utils/build-script -RT --reconfigure \
  --swift-vc-revision-include-dir="lib;include;tools"

Revision from whole repository excluding stdlib/ and docs/:

utils/build-script -RT --reconfigure \
  --swift-vc-revision-include-dir=".;:!stdlib;:!docs"

···

2016-05-07 1:23 GMT+09:00 Dmitri Gribenko <gribozavr@gmail.com>:

On Fri, May 6, 2016 at 9:16 AM, Jordan Rose <jordan_rose@apple.com> wrote:
> Hm. That might be a nice balance, and it’s not entirely a lie: that’s the
> version of the compiler that you’re using, if not the stdlib and runtime.
> I’d still like to put it behind a flag, so that we can turn it off it
> certain configurations, like #2105 attempted to do.
>
> Dmitri, any comments, since you caught the issue last time?

I think this would be a reasonable balance for local development. For
releases that CI produces I think this change will cause confusion
because the git hashes that are tagged would not match the git hashes
included in the compiler version.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/