Add support for loaded module trace emission #278
Conversation
|
@swift-ci test |
There was a problem hiding this comment.
One question about mutable state, but otherwise it's looking good. @varungandhi-apple might want to take a peek, but that can be post-merge review
| /// Path to the loaded module trace file, and whether it has been claimed by | ||
| /// a frontend job. It's important to track this because only one frontend | ||
| /// job should be used for module trace emission. | ||
| var loadedModuleTracePath: (VirtualPath, claimed: Bool)? |
There was a problem hiding this comment.
Mutable state, hmmm. Is there some way we could ensure that only one fronted job gets assigned this task without having this but if mutable state? E.g., have planning treat the first batch as special?
There was a problem hiding this comment.
Passing that info through compile planning adds a little bit of boilerplate, but yeah, it's probably best to avoid mutable state here.
|
@swift-ci test |
|
Are the trace tests being run with the new driver as well? Or were they already being run and failing? I'm not sure how the testing is structured there, I don't see anything in this PR adding something which makes sure those tests are run. |
They aren’t run in CI, but we have a mechanism to run them. So they were failing before. |
|
Yeah, |
Since the docs for this aren't open source, I'm not sure my test cases are all that great, but all the relevant integration tests are passing now and the implementation wasn't as complicated as I expected. The use of a "claimed" bit on
Driver.loadedModuleTracePathis definitely a bit of a hack though.Fixes:
Driver/loaded_module_trace.swift
Driver/loaded_module_trace_append.swift
Driver/loaded_module_trace_env.swift
Driver/loaded_module_trace_foundation.swift
Driver/loaded_module_trace_header.swift
Driver/loaded_module_trace_multifile.swift
Driver/loaded_module_trace_spi.swift