Prevent setup twice, use file coordinator for increased performance (#83

)

* Prevent setup twice, use file coordinator for increased writing performance

* Fix tests

Diagnostics.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>IDEDidComputeMac32BitWarning</key>
+	<true/>
+</dict>
+</plist>

DiagnosticsTests/Reporters/LogsReporterTests.swift

@@ -11,22 +11,22 @@ import XCTest
 
 final class LogsReporterTests: XCTestCase {
 
-    override func setUp() {
-        super.setUp()
-        try! DiagnosticsLogger.setup()
+    override func setUpWithError() throws {
+        try super.setUpWithError()
+        try DiagnosticsLogger.setup()
     }
 
-    override class func tearDown() {
-        try! DiagnosticsLogger.standard.deleteLogs()
-        super.tearDown()
+    override func tearDownWithError() throws {
+        try DiagnosticsLogger.standard.deleteLogs()
+        try super.tearDownWithError()
     }
 
     /// It should show logged messages.
     func testMessagesLog() {
         let message = UUID().uuidString
         DiagnosticsLogger.log(message: message)
         let diagnostics = LogsReporter.report().diagnostics as! String
-        XCTAssertTrue(diagnostics.contains(message))
+        XCTAssertTrue(diagnostics.contains(message), "Diagnostics is \(diagnostics)")
     }
 
     /// It should show errors.
@@ -41,13 +41,13 @@ final class LogsReporterTests: XCTestCase {
     }
 
     /// It should reverse the order of sessions to have the most recent session on top.
-    func testReverseSessions() {
+    func testReverseSessions() throws {
         DiagnosticsLogger.log(message: "first")
         DiagnosticsLogger.standard.startNewSession()
         DiagnosticsLogger.log(message: "second")
         let diagnostics = LogsReporter.report().diagnostics as! String
-        let firstIndex = diagnostics.range(of: "first")!.lowerBound
-        let secondIndex = diagnostics.range(of: "second")!.lowerBound
+        let firstIndex = try XCTUnwrap(diagnostics.range(of: "first")?.lowerBound)
+        let secondIndex = try XCTUnwrap(diagnostics.range(of: "second")?.lowerBound)
         XCTAssertTrue(firstIndex > secondIndex)
     }
 

Sources/DiagnosticsLogger.swift

@@ -20,7 +20,6 @@ public final class DiagnosticsLogger {
     static let standard = DiagnosticsLogger()
 
     private lazy var logFileLocation: URL = FileManager.default.documentsDirectory.appendingPathComponent("diagnostics_log.txt")
-    private var logFileHandle: FileHandle?
 
     private let inputPipe: Pipe = Pipe()
     private let outputPipe: Pipe = Pipe()
@@ -55,6 +54,9 @@ public final class DiagnosticsLogger {
     /// Sets up the logger to be ready for usage. This needs to be called before any log messages are reported.
     /// This method also starts a new session.
     public static func setup() throws {
+        guard !isSetUp() || standard.isRunningTests else {
+            return
+        }
         try standard.setup()
     }
 
@@ -98,9 +100,9 @@ extension DiagnosticsLogger {
             }
         }
 
-        logFileHandle = try FileHandle(forWritingTo: logFileLocation)
-        logFileHandle!.seekToEndOfFile()
-        logSize = Int64(logFileHandle!.offsetInFile)
+        let logFileHandle = try FileHandle(forWritingTo: logFileLocation)
+        logFileHandle.seekToEndOfFile()
+        logSize = Int64(logFileHandle.offsetInFile)
         setupPipe()
         setupCrashMonitoring()
         isSetup = true
@@ -161,7 +163,15 @@ extension DiagnosticsLogger {
             return nil
         }
 
-        return queue.sync { try? Data(contentsOf: logFileLocation) }
+        return queue.sync {
+            let coordinator = NSFileCoordinator(filePresenter: nil)
+            var error: NSError?
+            var logData: Data?
+            coordinator.coordinate(readingItemAt: logFileLocation, error: &error) { url in
+                logData = try? Data(contentsOf: url)
+            }
+            return logData
+        }
     }
 
     /// Removes the log file. Should only be used for testing purposes.
@@ -173,7 +183,7 @@ extension DiagnosticsLogger {
     private func log(message: String, file: String = #file, function: String = #function, line: UInt = #line) {
         guard isSetup else { return assertionFailure("Trying to log a message while not set up") }
 
-        self.queue.async { [unowned self] in
+        queue.async { [unowned self] in
             let date = self.formatter.string(from: Date())
             let file = file.split(separator: "/").last.map(String.init) ?? file
             let output = String(format: "%@ | %@:L%@ | %@\n", date, file, String(line), message)
@@ -182,19 +192,27 @@ extension DiagnosticsLogger {
     }
 
     private func log(_ output: String) {
+        // Make sure we have enough disk space left. This prevents a crash due to a lack of space.
+        guard Device.freeDiskSpaceInBytes > minimumRequiredDiskSpace else { return }
+
         guard
-            let data = output.data(using: .utf8),
-            let fileHandle = logFileHandle else {
+            let data = output.data(using: .utf8) else {
                 return assertionFailure("Missing file handle or invalid output logged")
         }
 
-        // Make sure we have enough disk space left. This prevents a crash due to a lack of space.
-        guard Device.freeDiskSpaceInBytes > minimumRequiredDiskSpace else { return }
-
-        fileHandle.seekToEndOfFile()
-        fileHandle.write(data)
-        logSize += Int64(data.count)
-        trimLinesIfNecessary()
+        let coordinator = NSFileCoordinator(filePresenter: nil)
+        var error: NSError?
+        coordinator.coordinate(writingItemAt: logFileLocation, error: &error) { [weak self] url in
+            guard let fileHandle = try? FileHandle(forWritingTo: url) else {
+                return
+            }
+            fileHandle.seekToEndOfFile()
+            fileHandle.write(data)
+            fileHandle.closeFile()
+
+            self?.logSize += Int64(data.count)
+            self?.trimLinesIfNecessary()
+        }
     }
 
     private func trimLinesIfNecessary() {