Skip to content

Conversation

@euanh
Copy link
Contributor

@euanh euanh commented Nov 28, 2024

copyTargetSwiftFromDocker() already copies all the same files as copyTargetSwift(),
and puts them in the destination directory. Calling copyTargetSwift() at the
end just results in several pointless rsync calls which sync the directories to
themselves.

Stopping in copyTargetSwift() shows that the source and destination arguments point to the same place:

Process 77272 stopped
* thread #2, queue = 'com.apple.root.default-qos.cooperative', stop reason = breakpoint 1.1 frame #0: ... at SwiftSDKGenerator+Copy.swift:105:8
   102      }
   103    }
   104
-> 105    func copyTargetSwift(from distributionPath: FilePath, sdkDirPath: FilePath) async throws {
   106      logGenerationStep("Copying Swift core libraries for the target triple into Swift SDK bundle...")
   107
   108      for (pathWithinPackage, pathWithinSwiftSDK) in [
Target 0: (swift-sdk-generator) stopped.
(lldb) print distributionPath.string
(String) ".../ubuntu-jammy.sdk/usr"
(lldb) print sdkDirPath.string
(String) ".../ubuntu-jammy.sdk"

The first rsync command line is then:

rsync -a .../ubuntu-jammy.sdk/usr/lib/swift .../ubuntu-jammy.sdk/usr/lib

Removing the call to copyTargetSwift() does not save much time because rsync does not do much work, however disentangling the Ubuntu-based SDK build from the container-based build makes it easier to refactor both of them independently.

Diffing SDKs built before and after this change shows them to be identical. The EndToEndTests (which currently can't be run by CI) continue to pass locally.

…TargetSwift

copyTargetSwiftFromDocker() already copies all the same files as copyTargetSwift,
and puts them in the destination directory.   Calling copyTargetSwift() at the
end just results in several pointless rsync calls which syncing directories to
themselves.

Stopping in copyTargetSwift() shows that the source and destination arguments
point to the same place:

    Process 77272 stopped
    * thread swiftlang#2, queue = 'com.apple.root.default-qos.cooperative', stop reason = breakpoint 1.1
        frame #0: ... at SwiftSDKGenerator+Copy.swift:105:8
       102      }
       103    }
       104
    -> 105    func copyTargetSwift(from distributionPath: FilePath, sdkDirPath: FilePath) async throws {
       106      logGenerationStep("Copying Swift core libraries for the target triple into Swift SDK bundle...")
       107
       108      for (pathWithinPackage, pathWithinSwiftSDK) in [
    Target 0: (swift-sdk-generator) stopped.
    (lldb) print distributionPath.string
    (String) ".../ubuntu-jammy.sdk/usr"
    (lldb) print sdkDirPath.string
    (String) ".../ubuntu-jammy.sdk"

The first rsync command line is then:

    rsync -a .../ubuntu-jammy.sdk/usr/lib/swift .../ubuntu-jammy.sdk/usr/lib

Removing the call to copyTargetSwift() does not save much time because rsync
does not do much work, however disentangling the Ubuntu-based SDK build from
the container-based build makes it easier to refactor both of them independently.

Diffing SDKs built before and after this change shows them to be identical.
The EndToEndTests (which currently can't be run by CI) continue to pass locally.
@euanh
Copy link
Contributor Author

euanh commented Nov 28, 2024

@swift-ci test

@euanh euanh changed the title generator/linux: copyTargetSwiftFromDocker does not need to call copy… generator/linux: copyTargetSwiftFromDocker does not need to call copyTargetSwift Nov 28, 2024
@euanh
Copy link
Contributor Author

euanh commented Nov 28, 2024

% SWIFT_SDK_GENERATOR_RUN_SLOW_TESTS=1 swift test --disable-swift-testing --filter EndToEndTests
...
Test Suite 'Swift60_UbuntuEndToEndTests' passed at 2024-11-28 15:21:24.124.
         Executed 4 tests, with 0 failures (0 unexpected) in 494.515 (494.515) seconds
Test Suite 'swift-sdk-generatorPackageTests.xctest' passed at 2024-11-28 15:21:24.124.
         Executed 18 tests, with 0 failures (0 unexpected) in 1959.080 (1959.081) seconds
Test Suite 'Selected tests' passed at 2024-11-28 15:21:24.124.
         Executed 18 tests, with 0 failures (0 unexpected) in 1959.080 (1959.082) seconds

@euanh euanh marked this pull request as ready for review November 28, 2024 15:22
@euanh euanh requested a review from MaxDesiatov as a code owner November 28, 2024 15:22
@MaxDesiatov MaxDesiatov added the performance Speeding up SDK generation label Nov 28, 2024
@euanh euanh merged commit 4c5d1a9 into swiftlang:main Nov 28, 2024
3 checks passed
@euanh euanh deleted the no-copytargetswift-for-container branch November 28, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Speeding up SDK generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants