Better progress reporting

MixinNetwork · Jun 14, 2023 · b150f10 · b150f10
1 parent b2683dc
commit b150f10
Show file tree

Hide file tree

Showing 20 changed files with 105 additions and 76 deletions.
diff --git a/Mixin.xcodeproj/project.pbxproj b/Mixin.xcodeproj/project.pbxproj
@@ -719,6 +719,7 @@
 		94341BFB2862F302009C9147 /* libopusenc.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = 94341BF72862F302009C9147 /* libopusenc.xcframework */; };
 		94341C002863530B009C9147 /* libogg.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = 94341BFF2862F454009C9147 /* libogg.xcframework */; };
 		9438252725EE697300709B7D /* CacheableAssetFileManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9438252625EE697300709B7D /* CacheableAssetFileManager.swift */; };
+		9439116A2A39EA4300CF6DC7 /* DeviceTransferProgress.swift in Sources */ = {isa = PBXBuildFile; fileRef = 943911692A39EA4300CF6DC7 /* DeviceTransferProgress.swift */; };
 		94396F2629EB11E300A57833 /* DeviceTransferProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94396F2529EB11E300A57833 /* DeviceTransferProtocol.swift */; };
 		94396F2829EB475500A57833 /* NWParameters+DeviceTransfer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94396F2729EB475500A57833 /* NWParameters+DeviceTransfer.swift */; };
 		94396F2A29EBE52400A57833 /* DeviceTransferClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94396F2929EBE52400A57833 /* DeviceTransferClient.swift */; };
@@ -1796,6 +1797,7 @@
 		94341BF72862F302009C9147 /* libopusenc.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; path = libopusenc.xcframework; sourceTree = "<group>"; };
 		94341BFF2862F454009C9147 /* libogg.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; path = libogg.xcframework; sourceTree = "<group>"; };
 		9438252625EE697300709B7D /* CacheableAssetFileManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CacheableAssetFileManager.swift; sourceTree = "<group>"; };
+		943911692A39EA4300CF6DC7 /* DeviceTransferProgress.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeviceTransferProgress.swift; sourceTree = "<group>"; };
 		94396F2529EB11E300A57833 /* DeviceTransferProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeviceTransferProtocol.swift; sourceTree = "<group>"; };
 		94396F2729EB475500A57833 /* NWParameters+DeviceTransfer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NWParameters+DeviceTransfer.swift"; sourceTree = "<group>"; };
 		94396F2929EBE52400A57833 /* DeviceTransferClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeviceTransferClient.swift; sourceTree = "<group>"; };
@@ -2869,6 +2871,7 @@
 				7C07A17C29D8FDBC00D4835C /* NetworkInterface.swift */,
 				940B30482A16050D00B45D26 /* NetworkSpeedInspector.swift */,
 				94149B422A17B4D5003E9E1A /* NetworkSpeedConditioner.swift */,
+				943911692A39EA4300CF6DC7 /* DeviceTransferProgress.swift */,
 			);
 			path = Utility;
 			sourceTree = "<group>";
@@ -5064,6 +5067,7 @@
 				9424C64B259246B600FFDAE0 /* main.swift in Sources */,
 				945278982626BCD600023A6C /* HighlightableButton.swift in Sources */,
 				7BEB5D9F22B79F5500B8B10E /* EmergencyContactLoginVerificationCodeViewController.swift in Sources */,
+				9439116A2A39EA4300CF6DC7 /* DeviceTransferProgress.swift in Sources */,
 				7CEB736429DBCBE5006FB5B2 /* DeviceTransferSnapshot.swift in Sources */,
 				7B59535122672D3500D59DB4 /* TopResultCell.swift in Sources */,
 				7CEB736629DBCD6A006FB5B2 /* DeviceTransferSticker.swift in Sources */,

diff --git a/Mixin/Service/DeviceTransfer/DeviceTransferClient.swift b/Mixin/Service/DeviceTransfer/DeviceTransferClient.swift
@@ -7,9 +7,9 @@ final class DeviceTransferClient {
 
     enum State {
         case idle
-        case transfer(progress: Double, speed: String)
+        case transfer(progress: Float, speed: String) // `progress` is between 0.0 and 1.0
         case failed(DeviceTransferError)
-        case importing(progress: Float)
+        case importing(progress: Float) // `progress` is between 0.0 and 1.0
         case finished
     }
 
@@ -29,13 +29,11 @@ final class DeviceTransferClient {
     private weak var statisticsTimer: Timer?
 
     private var fileStream: DeviceTransferFileStream?
-
-    // Access counts on main queue
-    private var processedCount = 0
-    private var totalCount: Int?
-
     private var messageProcessingObservers: Set<AnyCancellable> = []
 
+    // Access on main queue
+    private var progress = DeviceTransferProgress()
+
     private var opaquePointer: UnsafeMutableRawPointer {
         Unmanaged<DeviceTransferClient>.passUnretained(self).toOpaque()
     }
@@ -126,7 +124,7 @@ final class DeviceTransferClient {
 
     private func fail(error: DeviceTransferError) {
         assert(queue.isCurrent)
-        Logger.general.info(category: "DeviceTransferClient", message: "Stop: \(error) Processed: \(processedCount) Total: \(totalCount)")
+        Logger.general.info(category: "DeviceTransferClient", message: "Failed: \(error), progress: \(progress)")
         DispatchQueue.main.sync {
             self.statisticsTimer?.invalidate()
         }
@@ -139,7 +137,7 @@ final class DeviceTransferClient {
         state = .failed(error)
     }
 
-    private func startUpdatingProgressAndSpeed() {
+    private func startUpdatingStatistics() {
         assert(Queue.main.isCurrent)
         statisticsTimer?.invalidate()
         statisticsTimer = Timer.scheduledTimer(withTimeInterval: 1, repeats: true) { [weak self] timer in
@@ -149,12 +147,7 @@ final class DeviceTransferClient {
                 return
             }
             let speed = self.speedInspector.drain()
-            let progress: Double
-            if let totalCount = self.totalCount {
-                progress = Double(self.processedCount) * 100 / Double(totalCount)
-            } else {
-                progress = 0
-            }
+            let progress = self.progress.fractionCompleted
             self.queue.async {
                 guard case .transfer = self.state else {
                     DispatchQueue.main.sync(execute: timer.invalidate)
@@ -242,8 +235,8 @@ extension DeviceTransferClient {
             Logger.general.info(category: "DeviceTransferClient", message: "Total count: \(count)")
             self.state = .transfer(progress: 0, speed: "")
             DispatchQueue.main.async {
-                self.totalCount = count
-                self.startUpdatingProgressAndSpeed()
+                self.progress.totalUnitCount = count
+                self.startUpdatingStatistics()
             }
         case .finish:
             Logger.general.info(category: "DeviceTransferClient", message: "Received finish command")
@@ -279,7 +272,7 @@ extension DeviceTransferClient {
         assert(queue.isCurrent)
         DispatchQueue.main.sync {
             speedInspector.add(byteCount: content.count)
-            processedCount += 1
+            progress.completedUnitCount += 1
         }
 
         let firstHMACIndex = content.endIndex.advanced(by: -DeviceTransferProtocol.hmacDataCount)
@@ -331,7 +324,7 @@ extension DeviceTransferClient {
         DispatchQueue.main.sync {
             speedInspector.add(byteCount: content.count)
             if isReceivingNewFile {
-                processedCount += 1
+                progress.completedUnitCount += 1
             }
         }
 

diff --git a/Mixin/Service/DeviceTransfer/DeviceTransferMessageProcessor.swift b/Mixin/Service/DeviceTransfer/DeviceTransferMessageProcessor.swift
@@ -71,11 +71,8 @@ final class DeviceTransferMessageProcessor {
         }
     }
 
-    private var totalCount = 0
-    private var processedCount = 0
-
+    private var processingProgress = DeviceTransferProgress()
     private var writingCache: Cache?
-
     private var cacheReadingBuffer = Data(count: Int(bytesPerKiloByte))
 
     // Messages are saved to database in batch. See `messageSavingBatchCount`
@@ -107,7 +104,7 @@ final class DeviceTransferMessageProcessor {
         cache.handle.write(encryptedMessage)
         cache.wroteCount += encryptedMessage.count
         processingQueue.async {
-            self.totalCount += 1
+            self.processingProgress.totalUnitCount += 1
         }
 
         if cache.isOversized {
@@ -124,14 +121,14 @@ final class DeviceTransferMessageProcessor {
 
     func reportFileReceived() {
         processingQueue.async {
-            self.totalCount += 1
+            self.processingProgress.totalUnitCount += 1
         }
     }
 
     func finishProcessing() {
         assert(inputQueue.isCurrent)
         guard let lastCache = writingCache else {
-            Logger.general.info(category: "DeviceTransferMessageProcessor", message: "All pending messages are saved")
+            Logger.general.error(category: "DeviceTransferMessageProcessor", message: "No writing cache on finished")
             return
         }
         writingCache = nil
@@ -141,6 +138,7 @@ final class DeviceTransferMessageProcessor {
             try? fileManager.removeItem(at: lastCache.url)
             self.savePendingMessages()
             self.processFiles()
+            Logger.general.info(category: "DeviceTransferMessageProcessor", message: "Processing finished with progress: \(self.processingProgress)")
             self.progress = 1
         }
     }
@@ -157,9 +155,7 @@ extension DeviceTransferMessageProcessor {
 
     private func reportProgress() {
         assert(processingQueue.isCurrent)
-        // Divide the count as integer to prevent `progress` from rounding when counts are large
-        let progress = Float(processedCount * 100 / totalCount) / 100
-        self.progress = progress
+        self.progress = processingProgress.fractionCompleted
     }
 
     private func savePendingMessages() {
@@ -188,7 +184,7 @@ extension DeviceTransferMessageProcessor {
         }
 
         Logger.general.info(category: "DeviceTransferMessageProcessor", message: "Begin processing cache \(cache.index)")
-        var processedCountOnLastProgressReporting = self.processedCount
+        var completedCountOnLastProgressReporting = processingProgress.completedUnitCount
         while stream.hasBytesAvailable {
             guard !isCancelled else {
                 Logger.general.info(category: "DeviceTransferMessageProcessor", message: "Not processing cache \(cache.index) by cancellation")
@@ -236,9 +232,9 @@ extension DeviceTransferMessageProcessor {
                 } catch {
                     Logger.general.error(category: "DeviceTransferMessageProcessor", message: "Decrypt failed: \(error)")
                 }
-                processedCount += 1
-                if processedCount - processedCountOnLastProgressReporting == progressReportingInterval {
-                    processedCountOnLastProgressReporting = processedCount
+                processingProgress.completedUnitCount += 1
+                if processingProgress.completedUnitCount - completedCountOnLastProgressReporting == progressReportingInterval {
+                    completedCountOnLastProgressReporting = processingProgress.completedUnitCount
                     reportProgress()
                 }
             }
@@ -356,7 +352,7 @@ extension DeviceTransferMessageProcessor {
             return
         }
         Logger.general.info(category: "DeviceTransferMessageProcessor", message: "Start processing files")
-        var processedCountOnLastProgressReporting = processedCount
+        var processedCountOnLastProgressReporting = processingProgress.completedUnitCount
 
         for case let fileURL as URL in fileEnumerator {
             guard !isCancelled else {
@@ -394,9 +390,9 @@ extension DeviceTransferMessageProcessor {
                 }
             }
 
-            processedCount += 1
-            if processedCount - processedCountOnLastProgressReporting == progressReportingInterval {
-                processedCountOnLastProgressReporting = processedCount
+            processingProgress.completedUnitCount += 1
+            if processingProgress.completedUnitCount - processedCountOnLastProgressReporting == progressReportingInterval {
+                processedCountOnLastProgressReporting = processingProgress.completedUnitCount
                 reportProgress()
             }
 

diff --git a/Mixin/Service/DeviceTransfer/DeviceTransferServer.swift b/Mixin/Service/DeviceTransfer/DeviceTransferServer.swift
@@ -12,7 +12,7 @@ final class DeviceTransferServer {
     enum State {
         case idle
         case listening(hostname: String, port: UInt16)
-        case transfer(progress: Double, speed: String)
+        case transfer(progress: Float, speed: String) // `progress` is between 0.0 and 1.0
         case closed(ClosedReason)
     }
 

diff --git a/Mixin/Service/DeviceTransfer/DeviceTransferServerDataSource.swift b/Mixin/Service/DeviceTransfer/DeviceTransferServerDataSource.swift
@@ -25,7 +25,7 @@ final class DeviceTransferServerDataSource {
 // MARK: - Data Count
 extension DeviceTransferServerDataSource {
 
-    func totalCount() -> Int {
+    func totalCount() -> Int64 {
         assert(!Queue.main.isCurrent)
         let messagesCount = MessageDAO.shared.messagesCount()
         let attachmentsCount = attachmentsCount()
@@ -46,18 +46,18 @@ extension DeviceTransferServerDataSource {
         return total
     }
 
-    private func attachmentsCount() -> Int {
+    private func attachmentsCount() -> Int64 {
         let folders = AttachmentContainer.Category.allCases.map(\.pathComponent) + ["Transcript"]
-        let count = folders.reduce(0) { previousCount, folder in
+        let count: Int64 = folders.reduce(0) { previousCount, folder in
             let folderURL = AttachmentContainer.url.appendingPathComponent(folder)
             let count = validFileCount(in: folderURL)
             return previousCount + count
         }
         return count
     }
 
-    private func validFileCount(in url: URL) -> Int {
-        var count = 0
+    private func validFileCount(in url: URL) -> Int64 {
+        var count: Int64 = 0
         guard let fileEnumerator = FileManager.default.enumerator(at: url, includingPropertiesForKeys: [.isRegularFileKey], options: [.skipsHiddenFiles, .skipsPackageDescendants]) else {
             return 0
         }

diff --git a/Mixin/Service/DeviceTransfer/Utility/DeviceTransferProgress.swift b/Mixin/Service/DeviceTransfer/Utility/DeviceTransferProgress.swift
@@ -0,0 +1,36 @@
+import Foundation
+
+struct DeviceTransferProgress {
+
+    // NSProgress rounds `fractionCompleted` as binary, which may not be as
+    // expected from the view of decimal progress
+    // e.g. when `totalUnitCount` is `.max / 100`, and `completedUnitCount`
+    // is `(.max / 100) - 1`, the `fractionCompleted` will be 1.0, which may
+    // be considered as finished, or leads to misunderstanding
+
+    var totalUnitCount: Int64
+    var completedUnitCount: Int64
+
+    var fractionCompleted: Float {
+        if totalUnitCount == 0 {
+            return 0
+        } else {
+            // Currently provides 4 digits for precision, that is 0.01% ~ 100.0%
+            return Float(completedUnitCount * 10000 / totalUnitCount) / 10000
+        }
+    }
+
+    init(totalUnitCount: Int64 = 0, completedUnitCount: Int64 = 0) {
+        self.totalUnitCount = 0
+        self.completedUnitCount = 0
+    }
+
+}
+
+extension DeviceTransferProgress: CustomStringConvertible {
+
+    var description: String {
+        "<DeviceTransferProgress: \(completedUnitCount)/\(totalUnitCount)>"
+    }
+
+}
diff --git a/Mixin/UserInterface/Controllers/DeviceTransfer/DeviceTransferProgressViewController.swift b/Mixin/UserInterface/Controllers/DeviceTransfer/DeviceTransferProgressViewController.swift
@@ -194,8 +194,8 @@ extension DeviceTransferProgressViewController {
         }
     }
 
-    private func updateTitleLabel(with transferProgress: Double, speed: String) {
-        let progress = String(format: "%.2f", transferProgress)
+    private func updateTitleLabel(with transferProgress: Float, speed: String) {
+        let progress = String(format: "%.2f", transferProgress * 100)
         switch connection {
         case .server:
             titleLabel.text = R.string.localizable.transferring_chat_progress(progress)
@@ -204,7 +204,7 @@ extension DeviceTransferProgressViewController {
         case .cloud:
             break
         }
-        progressView.progress = Float(transferProgress / 100)
+        progressView.progress = transferProgress
         speedLabel.text = speed
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/AppDAO.swift b/MixinServices/MixinServices/Database/User/DAO/AppDAO.swift
@@ -44,8 +44,8 @@ public final class AppDAO: UserDatabaseDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func appsCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM apps")
+    public func appsCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM apps")
         return count ?? 0
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/AssetDAO.swift b/MixinServices/MixinServices/Database/User/DAO/AssetDAO.swift
@@ -113,8 +113,8 @@ public final class AssetDAO: UserDatabaseDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func assetsCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM assets")
+    public func assetsCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM assets")
         return count ?? 0
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/ConversationDAO.swift b/MixinServices/MixinServices/Database/User/DAO/ConversationDAO.swift
@@ -668,8 +668,8 @@ public final class ConversationDAO: UserDatabaseDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func conversationsCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM conversations")
+    public func conversationsCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM conversations")
         return count ?? 0
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/ExpiredMessageDAO.swift b/MixinServices/MixinServices/Database/User/DAO/ExpiredMessageDAO.swift
@@ -152,8 +152,8 @@ public final class ExpiredMessageDAO: UserDatabaseDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func expiredMessagesCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM expired_messages")
+    public func expiredMessagesCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM expired_messages")
         return count ?? 0
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/MessageDAO.swift b/MixinServices/MixinServices/Database/User/DAO/MessageDAO.swift
@@ -989,8 +989,8 @@ extension MessageDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func messagesCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM messages")
+    public func messagesCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM messages")
         return count ?? 0
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/MessageMentionDAO.swift b/MixinServices/MixinServices/Database/User/DAO/MessageMentionDAO.swift
@@ -24,8 +24,8 @@ public final class MessageMentionDAO: UserDatabaseDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func messageMentionsCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM message_mentions")
+    public func messageMentionsCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM message_mentions")
         return count ?? 0
     }
 

diff --git a/MixinServices/MixinServices/Database/User/DAO/ParticipantDAO.swift b/MixinServices/MixinServices/Database/User/DAO/ParticipantDAO.swift
@@ -184,8 +184,8 @@ public final class ParticipantDAO: UserDatabaseDAO {
         return db.select(with: sql, arguments: [limit])
     }
 
-    public func participantsCount() -> Int {
-        let count: Int? = db.select(with: "SELECT COUNT(*) FROM participants")
+    public func participantsCount() -> Int64 {
+        let count: Int64? = db.select(with: "SELECT COUNT(*) FROM participants")
         return count ?? 0
     }