swift-llbuild
swift-llbuild copied to clipboard
apple
Parallel Incremental Builds
This is my try at solving FB8394316 that I reported. I found no good way to know when it's safe to scan dependencies in parallel. If you scan all of them at once you end up with incorrect scan results. I ended up solving this with barriers between batches of dependencies. This passes all the tests and seems to work ok in Xcode. The barriers are created when a result is reported to a task as that should prevent calculations on stale values.
rdar://20248283
FYI, I haven't forgotten about this. Overall I really like the approach so far. Just digging through the build engine logic itself, amidst other distractions.
Apologies that this has sat in the queue for so long. It is not forgotten. I think it is ready to go, but would like to run some extra qualification runs before integrating.
Apologies that this has sat in the queue for so long. It is not forgotten. I think it is ready to go, but would like to run some extra qualification runs before integrating.
No problem. I think it needs more testing as I have been running this in Xcode for over a month now and I have noticed an issue that it can get into a state where it's not rebuilding things that have changed. I have observed the following:
- I think that it can only only happen after a build error (I have "Continue building after errors" unchecked)
- Restarting Xcode will let it discover and build the changed files correctly only once.
I have not had time to debug it, but maybe the symtoms give you a clue for what could be causing this?
Hey @erikolofsson, can you share the setup that you are using to run this in Xcode? I will be interested in debugging the issue that you described. cc: @dmbryson if you have any leads that can help with the above.
@erikolofsson @dmbryson @chiragramani It seems this has stalled out. Is there anything we can do to make progress on it?
Here's the patch on top of this PR if someone wants to try my not-so-nice solution
llbuild-fix-incremental-after-build-failure-patch.txt
In order to make it work inside Xcode 14.1 you need to replace llbuild.framework inside and then re-sign:
llbuild.frameworkXCBBuildService.frameworkXCBBuildService
@erikolofsson : Can you update this PR with the changes in your branch https://github.com/erikolofsson/swift-llbuild/tree/incremental-parallel-20248283-2
@dmbryson: Please re-review this diff. We have been testing this feature here at Uber and it is giving us an improved performance due to parallel module builds.
We did some local analysis to see the difference. The bug was that llbuild doesn’t scan rules until all the lanes are free; so it would not queue in more jobs even if all the inputs for a rule are available
Since the code was reviewed the last time three bugs were fixed:
- Cancelling builds didn't record the dependencies
- Dependency barriers were applied at the time of processing instead of at request, this caused lost build parallelism depending on timing
- When dependencies were checked in parallel outputs that were deleted in a dependency weren't detected
You can see differences here.
I have been running the latest code for two weeks without finding any new issues so I believe this is ready for review again.
The latest changes in the PR crash for us. https://github.com/erikolofsson/swift-llbuild/tree/incremental-parallel-20248283-2 works fine.
Thread 8 Crashed:: Dispatch queue: XCBBuildSystem.BuildManager.buildQueue
0 libsystem_pthread.dylib 0x1a8d43570 pthread_mutex_lock + 12
1 libc++.1.dylib 0x1a8c989e8 std::__1::mutex::lock() + 16
2 llbuild 0x102af9970 std::__1::lock_guard<std::__1::mutex>::lock_guard(std::__1::mutex&) + 8 (__mutex_base:90) [inlined]
3 llbuild 0x102af9970 std::__1::lock_guard<std::__1::mutex>::lock_guard(std::__1::mutex&) + 8 (__mutex_base:90) [inlined]
4 llbuild 0x102af9970 (anonymous namespace)::BuildEngineImpl::getTaskInfo(llbuild::core::Task*) + 36 (BuildEngine.cpp:1551)
5 llbuild 0x102af98c0 (anonymous namespace)::BuildEngineImpl::addTaskInputRequest(llbuild::core::Task*, llbuild::core::KeyType const&, unsigned long, bool, bool) + 52 (BuildEngine.cpp:1815)
6 llbuild 0x102a76874 llb_buildsystem_command_interface_task_needs_input + 52 (BuildSystem-C-API.cpp:1215)
7 XCBBuildSystem 0x10234d5c8 protocol witness for DynamicTaskExecutionDelegate.requestInputNode(node:nodeID:) in conformance OperatorSystemAdaptorDynamicContext + 100
8 XCBTaskExecution 0x102ddde44 SwiftDriverJobSchedulingTaskAction.taskSetup(_:dynamicExecutionDelegate:) + 528
9 XCBBuildSystem 0x10235e0f0 specialized InProcessCommand.start(_:_:) + 152
10 llbuild 0x102a89b68 specialized CommandWrapper.configure(_:_:_:_:) + 1296 (BuildSystemBindings.swift:409)
11 llbuild 0x102a84c50 CommandWrapper.configure(_:_:_:_:) + 16 [inlined]
12 llbuild 0x102a84c50 closure #1 in ToolWrapper.buildCommand(_:_:) + 92 (BuildSystemBindings.swift:202)
13 llbuild 0x102a76694 (anonymous namespace)::CAPIExternalCommand::CAPIExternalCommand(llvm::StringRef, llb_buildsystem_external_command_delegate_t_) + 200 (BuildSystem-C-API.cpp:1059) [inlined]
14 llbuild 0x102a76694 (anonymous namespace)::CAPIExternalCommand::CAPIExternalCommand(llvm::StringRef, llb_buildsystem_external_command_delegate_t_) + 200 (BuildSystem-C-API.cpp:1039) [inlined]
15 llbuild 0x102a76694 llb_buildsystem_external_command_create + 264 (BuildSystem-C-API.cpp:1171)
16 llbuild 0x102a84a54 ToolWrapper.buildCommand(_:_:) + 320 (BuildSystemBindings.swift:221)
17 llbuild 0x102a88c48 closure #1 in ToolWrapper.createCommand(_:) + 20 (BuildSystemBindings.swift:176) [inlined]
18 llbuild 0x102a88c48 specialized Optional.map<A>(_:) + 20 [inlined]
19 llbuild 0x102a88c48 ToolWrapper.createCommand(_:) + 32 (BuildSystemBindings.swift:176) [inlined]
20 llbuild 0x102a88c48 closure #1 in BuildSystem.lookupTool(_:) + 172 (BuildSystemBindings.swift:1192)
21 llbuild 0x102a77ee4 (anonymous namespace)::CAPITool::createCommand(llvm::StringRef) + 36 (BuildSystem-C-API.cpp:685)
22 llbuild 0x102b0f260 (anonymous namespace)::BuildFileImpl::parseCommandsMapping(llvm::yaml::MappingNode*) + 528 (BuildFile.cpp:815) [inlined]
23 llbuild 0x102b0f260 (anonymous namespace)::BuildFileImpl::parseRootNode(llvm::yaml::Node*) + 6580 (BuildFile.cpp:434)
24 llbuild 0x102b0cd7c (anonymous namespace)::BuildFileImpl::load() + 228 (BuildFile.cpp:1015) [inlined]
25 llbuild 0x102b0cd7c llbuild::buildsystem::BuildFile::load() + 272 (BuildFile.cpp:1062)
26 llbuild 0x102b1288c (anonymous namespace)::BuildSystemImpl::loadDescription(llvm::StringRef) + 76 (BuildSystem.cpp:303) [inlined]
27 llbuild 0x102b1288c llbuild::buildsystem::BuildSystem::loadDescription(llvm::StringRef) + 104 (BuildSystem.cpp:4053)
28 llbuild 0x102b06978 (anonymous namespace)::BuildSystemFrontendImpl::initialize() + 268 (BuildSystemFrontend.cpp:390)
29 llbuild 0x102b06dac (anonymous namespace)::BuildSystemFrontendImpl::build(llvm::StringRef) + 8 (BuildSystemFrontend.cpp:468) [inlined]
30 llbuild 0x102b06dac llbuild::buildsystem::BuildSystemFrontend::build(llvm::StringRef) + 36 (BuildSystemFrontend.cpp:819)
31 llbuild 0x102a88b6c BuildSystem.build(target:) + 152 (BuildSystemBindings.swift:1096)
32 XCBBuildSystem 0x102349afc BuildOperation.build() + 9180
33 XCBBuildSystem 0x10236215c closure #2 in BuildManager.startBuild(_:) + 60
34 XCBBuildSystem 0x1023616d0 thunk for @escaping @callee_guaranteed () -> () + 28
35 libdispatch.dylib 0x1a8bd163c _dispatch_block_async_invoke2 + 148
36 libdispatch.dylib 0x1a8bc2504 _dispatch_client_callout + 20
37 libdispatch.dylib 0x1a8bc5994 _dispatch_continuation_pop + 504
38 libdispatch.dylib 0x1a8bc4ffc _dispatch_async_redirect_invoke + 584
39 libdispatch.dylib 0x1a8bd3f94 _dispatch_root_queue_drain + 396
40 libdispatch.dylib 0x1a8bd47c0 _dispatch_worker_thread2 + 164
41 libsystem_pthread.dylib 0x1a8d450c4 _pthread_wqthread + 228
42 libsystem_pthread.dylib 0x1a8d43e20 start_wqthread + 8
I don't think the other commits on main branch are compatible with released Xcode. There shouldn't be any differences in my code between those branches. If you delete the commits after efb176d866e3e99855a5b799da224a9f95058f2e and only include the pull request commits it should work.