The OSX libtool wrapper increases incremental build times

[michaeleisel]

tools/objc/libtool.sh increases the incremental build time for a single-target app with 700 implementation files by ~3 seconds. It appears that /usr/bin/libtool takes only a fraction of a second, but the work being done beforehand with symlinks and hashing takes longer. This work is to prevent giving libtool object files with the same basename. A few things:

• libtool seems to just throw a warning, rather than error out. Is there really a problem here?
• If so, which parts of this can we get rid of? E.g., reproducible hashing, symlinks for all files rather than just the duplicates, etc.

[michaeleisel]

@c-parsons can you provide some context on this?

[keith]

I would definitely be curious to know what the bug mentioned here is (or was):

bazel/tools/objc/libtool.sh

Lines 61 to 64 in 6326ff8

	# Libtool artifact symlinking. Apple's libtool has a bug when there are two
	# input files with the same basename. We thus create symlinks that are named
	# with a hash suffix for each input, and pass them to libtool.
	# See b/28186497.

[kastiglione]

Basenames are enforced to be unique per module by swiftc.

% swiftc input.swift foo/input.swift 
<unknown>:0: error: filename "input.swift" used twice: 'input.swift' and 'foo/input.swift'
<unknown>:0: note: filenames are used to distinguish private declarations with the same name

In which case, this behavior seems to be pure overhead for swift_library archives.

[c-parsons]

This change was from several years ago, so take my recollection with a grain of salt.
If I recall correctly, these libtool warnings for same-basename files was problematic for Xcode integration when compiling objective C sources. Given your findings on build performance, we should maybe revisit this.

[michaeleisel]

So we could either:

• detect if everything is unique, in which case take a fast path
• only for those that are not unique do this work
• try to optimize the specific operations being done

I don't like working in bash, so if I'm implementing it, I'm inclined towards the first option, especially because it couldn't have been working for people in Xcode anyways if they had duplicate basenames. But I'm curious what people would like.

[allevato]

cc @trybka who may have some thoughts on improvements to this script.

[michaeleisel]

Another alternative that @keith brought up is to provide some ability to just disable this whole thing and run /usr/bin/libtool directly

[michaeleisel]

@trybka ping. i'm happy to rewrite the current rule in c++, as i believe that will solve the performance issues here, if you're fine with that as well

[trybka]

The warning isn't spurious: It picks one and omits the other, which is ... problematic.

If you use xcodebuild, then it'll name outputs foo.o, foo0.o, etc. which does avoid the problem.

In Bazel, we had to resort to this ... workaround.

The current functionality (hashing/symlinking) is actually in Python: tools/objc/make_hashed_objlist.py
I have no objection to rewriting it in C++, and I think any of the optimizations you mentioned (detect unique and skip, only rename non-unique, make individual operations faster) are all fine with me.

[michaeleisel]

@trybka do we need the hash to be deterministic? bit more work in c++ if we have to add in md5 support

[michaeleisel]

@trybka PR opened: #11958

[trybka]

I do believe it needs to be deterministic, otherwise it will invalidate caches.

If archives can be generated which contain bit-for-bit identical object files, but the names differ, that is a problem.

That "different" archive will be detected by Bazel to be a new input, and will cause re-linking to potentially occur, invalidating caches.

[michaeleisel]

this is now done, released in 3.5 (with a fix for a regression it introduced in 3.6 rc4). @trybka let's close?

[keith]

I don't think it made 3.5? 3.6 is the first release with both I think?

[michaeleisel]

ah is that right? (we've been on a fork with both anyways). in any case, this seems resolved

[trybka]

I don't think I have any specific bits to close these?

@oquenchil probably does.