Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resume building releases with Xcode 13 #7426

Merged
merged 4 commits into from
Sep 3, 2021
Merged

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Sep 3, 2021

#7401 worked around the problem so we want to start packaging it again. I also fixed the remaining things making cocoa-pipeline fail now that the cocoapods release required for that is out, and this branch built successfully: https://ci.realm.io/blue/organizations/jenkins/cocoa-pipeline/detail/cocoa-pipeline/1497/pipeline/

We already did this for non-SPM builds as it makes the build ~20% faster and
write about half as much data.
This has been broken ever since we started hiding the project file from
Carthage because sed can't do in-place edits on a symlink (but ex can).
@tgoyne tgoyne self-assigned this Sep 3, 2021
Comment on lines +382 to +395
let str: String
switch self {
case let .decimal128(d):
return d
case let .int64(i):
return try? Decimal128(string: String(i))
str = String(i)
case let .int32(i):
return try? Decimal128(string: String(i))
str = String(i)
case let .double(d):
return try? Decimal128(string: String(d))
str = String(d)
default:
return nil
}
return try? Decimal128(string: str)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change made this function compile in 5ms rather than 150ms (as reported by -debug-time-function-bodies)

public func distance(to other: Decimal128) -> Stride {
other - self
public func distance(to other: Decimal128) -> Decimal128 {
unsafeDowncast(other.decimalNumber(bySubtracting: self), to: Decimal128.self)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overloaded operators take a really long time to type-check so each of these were taking close to 100ms to build and now are <1ms. unsafeDowncast() ended up not speeding up the build but made the release binary a bit smaller (but not enough to justify listing it in the changelog).

Copy link
Contributor

@leemaguire leemaguire Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding some comments to say why you use unsafeDowncast? Its not initially apparent that it improves binary size.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I suppose you could just always trace the git blame

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference between unsafeDowncast() and as! is that the latter inserts runtime checks to verify that it actually has the type, while unsafeDowncast() doesn't in optimized builds. Those checks ended up being a significant percentage of this file due to how little the Swift wrapper actually does, but in absolute terms they weren't large.

@@ -165,7 +165,7 @@ public extension ObjectiveCSupport {
return convertBson(object: value)
}
return .null
}.map { $0 == .null ? nil : $0 })
}.map { (v: AnyBSON) -> AnyBSON? in v == .null ? nil : v })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly this type hint made this function a lot faster to build.

@@ -649,8 +649,7 @@ case "$COMMAND" in
export ASAN_OPTIONS='check_initialization_order=true:detect_stack_use_after_return=true'
fi
xcrun swift package resolve
find .build -name views.cpp -delete
xcrun swift test --configuration "$(echo "$CONFIGURATION" | tr "[:upper:]" "[:lower:]")" $SANITIZER
xcrun swift test -Xcc -g0 --configuration "$(echo "$CONFIGURATION" | tr "[:upper:]" "[:lower:]")" $SANITIZER
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-SPM pull request jobs were already disabling debug symbols so this brings SPM in line with them. This knocks ~20% off the build time and cuts the total size of all the object files and such in half, which might fix some of the jobs which were timing out.

@@ -106,7 +106,7 @@ xctest() {
)
elif [[ $NAME == SwiftPackageManager* ]]; then
if [ -n "$sha" ]; then
sed -i '' 's@branch = "master"@branch = "'"$sha"'"@' "$DIRECTORY/$NAME.xcodeproj/project.pbxproj"
ex '+%s@branch = "master"@branch = "'"$sha"'"@' -scwq "$DIRECTORY/$NAME.xcodeproj/project.pbxproj"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sed -i doesn't work on symlinks and this file has been a symlink for a while. ex (the line-editor part of vim) does and is a reasonably drop-in replacement.

@@ -133,7 +133,9 @@ xctest() {
local scheme=(-scheme "$NAME")

# Ensure that dynamic framework tests try to use the correct version of the prebuilt libraries.
sed -i '' 's@/realm-swift-latest@/realm-swift-latest/'"${REALM_XCODE_VERSION}"'@' "$DIRECTORY/$NAME.xcodeproj/project.pbxproj"
if grep '/realm-swift-latest' "$DIRECTORY/$NAME.xcodeproj/project.pbxproj"; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike sed, ex reports failure if the expression isn't present in the file so check first.

@tgoyne tgoyne requested review from leemaguire and jsflax and removed request for leemaguire September 3, 2021 03:07
Copy link
Contributor

@jsflax jsflax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool and the compilation improvements are unexpected. Did you run -debug-time-function-bodies on the entire codebase?

Copy link
Contributor

@leemaguire leemaguire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned a fair bit from this 👍🏼

@tgoyne
Copy link
Member Author

tgoyne commented Sep 3, 2021

Did you run -debug-time-function-bodies on the entire codebase?

Yes, and the main thing is showed was that the Swift frontend isn't a meaningful portion of our build time and there isn't much room for improvement there. Everything in non-test code left that's >10ms adds up to a total of ~300ms and most of that is in the long tail of 12ms functions. None of the test code appears to be hitting any particularly bad cases for the compiler either (at least with Xcode 13).

@tgoyne tgoyne merged commit 59a36ee into master Sep 3, 2021
@tgoyne tgoyne deleted the tg/build-xcode-13-release branch September 3, 2021 15:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants