Wrap various convert subtasks in signpost intervals #1112
Conversation
d-ronnqvist
force-pushed
the
signposts-and-logging
branch
from
621a6ee to
ac8a510
Compare
|
@swift-ci please test |
d-ronnqvist
changed the title
d-ronnqvist
changed the title
There was a problem hiding this comment.
Looks great!
It occurred to me while reading these signposts that they wrap important steps in the compilation process. A newcomer trying to understand how DocC works might want to refer to these to understand the overall process a bit better.
I'm imaging an ASCII art diagram of these signposts, or better yet maybe a screenshot of how they appear in Instruments, could appear in the DocC documentation or contribution guide somewhere, explaining the overall process and order of operations for a DocC build.
| @@ -19,8 +19,8 @@ let swiftSettings: [SwiftSetting] = [ | |||
| let package = Package( | |||
| name: "SwiftDocC", | |||
| platforms: [ | |||
| .macOS(.v10_15), | |||
| .iOS(.v13) | |||
| .macOS(.v12), | |||
There was a problem hiding this comment.
Is there any implication here for who can run DocC? Or is DocC already associated with a certain version of Swift that precludes older versions of MacOS and iOS anyway?
There was a problem hiding this comment.
Yes, but not in practice.
Since most developers use the DocC that comes with Xcode—rather than build DocC from source—this doesn't change what version of macOS they need to use because Xcode requires a newer version of macOS than what DocC does. Xcode 16.1 (the latest release as of this writing) requires macOS 14.5 and the last release of Xcode to support macOS 12 was Xcode 13.4.1.
Linux or Windows are unaffected by this. It is possible that someone would build DocC from source and run it directly (not through Xcode or Swift Package Manager) on older macOS versions in which case they'd need to pin to the 6.1 release branch for the last version of DocC to support those macOS versions.
There was a problem hiding this comment.
@franklinsch and I talked about this in the past and didn't have any concerns about increasing these platform versions.
| @@ -83,6 +83,7 @@ public typealias BundleIdentifier = String | |||
| /// - ``parents(of:)`` | |||
| /// | |||
| public class DocumentationContext { | |||
| private let signposter = ConvertActionConverter.signposter | |||
There was a problem hiding this comment.
Should we pass this into the initializer? ...instead of silently coupling this to ConvertActionConverter like this?
Or if you'd prefer to avoid the noise in all the initializers, we could use a global, singleton class/value instead of referring to ConvertActionConverter.
There was a problem hiding this comment.
This is very much an implementation detail so it shouldn't show up in initializers or other API. A global would be one option but preferred to namespace it under a type instead of putting that information in the variable name.
| } | ||
|
|
||
| return (mimeType as NSString) as String | ||
| return UTType(filenameExtension: ext)?.preferredMIMEType ?? defaultMimeType |
There was a problem hiding this comment.
Unrelated improvement?
There was a problem hiding this comment.
Yes, this was a deprecation warning (see the 7f7b106 commit message) once we increased the platform versions to macOS 12 / iOS 15.
- Add signpost intervals around a few more tasks
|
@swift-ci please test |
Yes, I think this information could serve as useful data for an illustration in the contributor documentation. We'd probably want some extra content to talk about the phases in more detail, so I think I'd prefer to add this in another PR. |
rdar://141210010 * Add a no-op shim for os.OSSignpost * Bump platform versions to be able to use OSSignposter * Fix deprecation warnings about UTType API * Wrap various major convert tasks in signpost intervals * Add a no-op shim for os.Logger * Add support for interpolating errors in no-op log messages * Address code review feedback: - Add signpost intervals around a few more tasks
* Wrap various convert subtasks in signpost intervals (#1112) rdar://141210010 * Add a no-op shim for os.OSSignpost * Bump platform versions to be able to use OSSignposter * Fix deprecation warnings about UTType API * Wrap various major convert tasks in signpost intervals * Add a no-op shim for os.Logger * Add support for interpolating errors in no-op log messages * Address code review feedback: - Add signpost intervals around a few more tasks * Update benchmark tool to same macOS version as DocC (#1124) rdar://141185270
Bug/issue #, if applicable:
Summary
This adds macOS signpost intervals around various major convert tasks. This information can be helpful to cross reference other information against when working on performance, memory usage, etc.
To make it easier to add signpost intervals without repeating
#if canImport(os)throughout the implementation, I made a no-op shim for the signposter type so that the call site doesn't need to check if it can emit an interval.I made a similar shim for logging although nothing uses it to log yet.
Dependencies
None.
Testing
This isn't a user-facing change but you can verify the low-level developer output on macOS by running
log stream --predicate 'subsystem=="org.swift.docc"' --signpostin one Terminal window and then running anydocc convertcommand in another Terminal window.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded[ ] Updated documentation if necessary