Skip to content

Commit

Permalink
Don't write RenderJSON atomically
Browse files Browse the repository at this point in the history
By default Swift-DocC writes all files to disk atomically. This
is generally a good default since it can prevent confusing errors. When
writing atomically, the file will first be written to a temp location
and then, only when the writing is complete, copied into the final
destination. This means that if writing fails halfway through, there
won't be a malformed file in the destination. If the file is there at
all, it's correct.

However, for writing RenderJSON from `docc convert` specifically,
this is unnecessary for a couple of reasons.

1. The `docc convert` process never reads RenderJSON, only writes it.
2. `docc convert` already writes _all_ of its output to a temp directory
   and only copies it over to the final destination if the entire
   conversion ran without errors.

So in the event that a RenderJSON write _does_ fail halfway through, we
still end up with the desired behavior of not having malformed
JSON in the target destination. An error would be thrown and the
conversion would fail without copying any of the bad output from the
temp directory. As it stands `docc convert` effectively implements
atomic writes twice.

The reason to _not_ write RenderJSON atomically is performance.
By writing non-atomically there's an ~8% performance win for the
"documentation-processing" step of the conversion process.
  • Loading branch information
ethan-kusters committed May 25, 2022
1 parent 99776d6 commit 3e496c7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class JSONEncodingRenderNodeWriter {
let encoder = RenderJSONEncoder.makeEncoder()

let data = try renderNode.encodeToJSON(with: encoder, renderReferenceCache: renderReferenceCache)
try fileManager.createFile(at: renderNodeTargetFileURL, contents: data)
try fileManager.createFile(at: renderNodeTargetFileURL, contents: data, options: nil)

guard let indexHTML = transformForStaticHostingIndexHTML else {
return
Expand Down
17 changes: 17 additions & 0 deletions Sources/SwiftDocCUtilities/Utility/FileManagerProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ protocol FileManagerProtocol {
///
/// - Throws: If the file couldn't be created with the specified contents.
func createFile(at: URL, contents: Data) throws

/// Creates a file with the given contents at the given url with the specified
/// writing options.
///
/// - Parameters:
/// - at: The location to create the file
/// - contents: The data to write to the file.
/// - options: Options for writing the data. Provide `nil` to use the default
/// writing options of the file manager.
func createFile(at location: URL, contents: Data, options writingOptions: NSData.WritingOptions?) throws
}

extension FileManagerProtocol {
Expand All @@ -81,4 +91,11 @@ extension FileManager: FileManagerProtocol {
try contents.write(to: location, options: .atomic)
}

func createFile(at location: URL, contents: Data, options writingOptions: NSData.WritingOptions?) throws {
if let writingOptions = writingOptions {
try contents.write(to: location, options: writingOptions)
} else {
try contents.write(to: location)
}
}
}
4 changes: 4 additions & 0 deletions Tests/SwiftDocCUtilitiesTests/Utility/TestFileSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProvider {
}
}

func createFile(at url: URL, contents: Data, options: NSData.WritingOptions?) throws {
try createFile(at: url, contents: contents)
}

func contents(atPath: String) -> Data? {
filesLock.lock()
defer { filesLock.unlock() }
Expand Down

0 comments on commit 3e496c7

Please sign in to comment.