Big NoteArchive refactor

Moves to a model where the NoteArchive can cache modified page content, later update their properties, and explicitly create versions after accumulating a lot of cached changes.

The goal is to enable faster propagation of changes beween views. In the previous architecture, changes didn't hit NoteArchive from NoteArchiveDocument until the autosave timer fired, which is way too slow.

Author: bdewey

CommonplaceBookApp/NoteArchive.swift

@@ -38,16 +38,21 @@ public struct NoteArchive {
   /// Mapping of page UUID (constant across revisions) to the current page properties digest
   private var pagePropertyDigests: [String: String] = [:]
 
+  /// A mapping of page UUID to page contents loaded from the archive.
+  private var pageContentsCache: [String: PageContents] = [:]
+
   /// Returns the current mapping of page id to page properties
   public var pageProperties: [String: PageProperties] {
-    return pagePropertyDigests.compactMapValues { propertyDigest -> PageProperties? in
+    let archiveVersion = pagePropertyDigests.compactMapValues { propertyDigest -> PageProperties? in
       guard
         let snippet = archive.snippets[propertyDigest],
         let properties = try? PageProperties(snippet) else {
         return nil
       }
       return properties
     }
+    let cacheVersion = pageContentsCache.compactMapValues { $0.pageProperties }
+    return archiveVersion.merging(cacheVersion, uniquingKeysWith: { _, new in new })
   }
 
   public enum SerializationError: Error {
@@ -84,6 +89,7 @@ public struct NoteArchive {
 
   /// Text version of the archive, suitable for storing to disk.
   public func textSerialized() -> String {
+    assert(pageContentsCache.allSatisfy({ !$0.value.dirty }))
     return archive.textSerialized()
   }
 
@@ -93,27 +99,27 @@ public struct NoteArchive {
   @discardableResult
   public mutating func insertNote(
     _ text: String,
-    contentChangeTime timestamp: Date,
-    versionTimestamp: Date
+    contentChangeTime timestamp: Date
   ) throws -> String {
-    let (propertiesSnippet, _) = try archivePageProperties(from: text, timestamp: timestamp)
+    var pageContents = PageContents()
+    pageContents.setText(text, modifiedTimestamp: timestamp)
     let key = UUID().uuidString
-    pagePropertyDigests[key] = propertiesSnippet.sha1Digest
-    try archivePageManifestVersion(timestamp: versionTimestamp)
+    pageContentsCache[key] = pageContents
     return key
   }
 
   /// Removes a note from the archive.
   /// - throws: `RetrievalError.noSuchPage` if the page does not exist.
-  public mutating func removeNote(for pageIdentifier: String, versionTimestamp: Date) throws {
-    guard pagePropertyDigests.removeValue(forKey: pageIdentifier) != nil else {
-      throw RetrievalError.noSuchPage(pageIdentifier)
-    }
-    try archivePageManifestVersion(timestamp: versionTimestamp)
+  public mutating func removeNote(for pageIdentifier: String) {
+    pageContentsCache.removeValue(forKey: pageIdentifier)
+    pagePropertyDigests.removeValue(forKey: pageIdentifier)
   }
 
   /// Gets the current version of the text for a particular page.
   public func currentText(for pageIdentifier: String) throws -> String {
+    if let cacheEntry = pageContentsCache[pageIdentifier] {
+      return cacheEntry.text
+    }
     let properties = try currentPageProperties(for: pageIdentifier).properties
     guard let noteSnippet = archive.snippets[properties.sha1Digest] else {
       throw RetrievalError.noSuchText(properties.sha1Digest)
@@ -141,36 +147,55 @@ public struct NoteArchive {
   /// - parameter pageIdentifier: The page identifier to update
   /// - parameter text: The new text of the page
   /// - parameter contentChangeTime: The *content change* timestamp of the text
-  /// - parameter versionTimestamp: The time we are applying this change. If nil, the version
-  ///             will be recorded with `timestamp`. Importing an existing file is the expected
-  ///             case where the version timestamp is different from the content change timestamp.
   /// - note: If `text` is not different from the current value associated with `pageIdentifier`,
   ///         this operation is a no-op. No new version gets created.
   public mutating func updateText(
     for pageIdentifier: String,
     to text: String,
-    contentChangeTime timestamp: Date,
-    versionTimestamp: Date
-  ) throws {
-    let (existingSnippet, existingProperties) = try currentPageProperties(for: pageIdentifier)
-    let (newSnippet, newProperties) = try archivePageProperties(from: text, timestamp: timestamp)
-    // New content is the same as the old content
-    if newProperties.sha1Digest == existingProperties.sha1Digest {
+    contentChangeTime timestamp: Date
+  ) {
+    if pageContentsCache[pageIdentifier] != nil {
+      pageContentsCache[pageIdentifier]!.setText(text, modifiedTimestamp: timestamp)
       return
+    } else {
+      var contents = PageContents()
+      contents.setText(text, modifiedTimestamp: timestamp)
+      pageContentsCache[pageIdentifier] = contents
     }
-    existingSnippet.encodeAsDiff(from: newSnippet)
-    guard let existingTextSnippet = archive.snippets[existingProperties.sha1Digest] else {
-      throw RetrievalError.noSuchPage(existingProperties.sha1Digest)
+  }
+
+  /// Updates all page properties that are stale in the contents cache.
+  /// - returns: How many page properties were updated.
+  @discardableResult
+  public mutating func batchUpdatePageProperties() -> Int {
+    let updated = archive.updatePageProperties(
+      in: pageContentsCache.filter({ $0.value.pagePropertiesStale }),
+      parsingRules: parsingRules
+    )
+    pageContentsCache.merge(updated, uniquingKeysWith: { _, new in new })
+    return updated.count
+  }
+
+  /// Creates a new Version representing the current page manifest in the archive.
+  /// - parameter timestamp: The version timestamp.
+  /// - throws: Any errors creating the symbolic reference
+  public mutating func archivePageManifestVersion(timestamp: Date) throws {
+    try flushContentsCache()
+    let version = Version(timestamp: timestamp, digest: archivePageManifest())
+    if let existingVersion = pagePropertiesVersionHistory.last,
+      existingVersion.digest == version.digest {
+      // The new version is identical to the old version -- no-op.
+      return
     }
-    guard let newTextSnippet = archive.snippets[newProperties.sha1Digest] else {
-      throw RetrievalError.noSuchPage(newProperties.sha1Digest)
+    if let existingVersion = pagePropertiesVersionHistory.last,
+      let oldManifestSnippet = archive.snippets[existingVersion.digest],
+      let newManifestSnippet = archive.snippets[version.digest] {
+      newManifestSnippet.encodeAsDiff(from: nil)
+      oldManifestSnippet.encodeAsDiff(from: newManifestSnippet)
     }
-    newTextSnippet.encodeAsDiff(from: nil)
-    existingTextSnippet.encodeAsDiff(from: newTextSnippet)
-    pagePropertyDigests[pageIdentifier] = newSnippet.sha1Digest
-    try archivePageManifestVersion(timestamp: versionTimestamp)
+    pagePropertiesVersionHistory.append(version)
+    try archiveVersionHistory()
   }
-
 }
 
 // MARK: - Import
@@ -188,15 +213,13 @@ public extension NoteArchive {
         try updateText(
           for: importRecord.pageIdentifier,
           to: text,
-          contentChangeTime: contentChangeDate,
-          versionTimestamp: importDate
+          contentChangeTime: contentChangeDate
         )
       }
     } else {
       let pageIdentifier = try insertNote(
         text,
-        contentChangeTime: contentChangeDate,
-        versionTimestamp: importDate
+        contentChangeTime: contentChangeDate
       )
       importRecords[fileName] = FileImportRecord(
         pageIdentifier: pageIdentifier,
@@ -235,6 +258,39 @@ private extension NoteArchive {
     }
   }
 
+  /// An in-memory cache record of the contents of a page
+  struct PageContents {
+    var text: String
+    var modifiedTimestamp: Date
+    var dirty: Bool
+    var pageProperties: PageProperties?
+    var pagePropertiesStale: Bool
+
+    init() {
+      self.text = ""
+      self.modifiedTimestamp = Date.distantPast
+      self.dirty = false
+      self.pageProperties = nil
+      self.pagePropertiesStale = false
+    }
+
+    init(text: String, pageProperties: PageProperties) {
+      self.text = text
+      self.dirty = false
+      self.pageProperties = pageProperties
+      self.pagePropertiesStale = false
+      self.modifiedTimestamp = pageProperties.timestamp
+    }
+
+    /// Updates text.
+    mutating func setText(_ text: String, modifiedTimestamp: Date) {
+      self.text = text
+      self.modifiedTimestamp = modifiedTimestamp
+      self.dirty = true
+      self.pagePropertiesStale = true
+    }
+  }
+
   /// Represents a specific file that has been imported into the archive.
   struct FileImportRecord: Codable {
     /// The UUID representing the page that holds the file contents.
@@ -257,6 +313,53 @@ private extension NoteArchive {
     try archive.setSymbolicReference(key: "file-import", text: encoded)
   }
 
+  /// Writes any dirty content from `pageContentsCache` to `archive`
+  /// - returns: How many modified pages were updated
+  @discardableResult
+  mutating func flushContentsCache() throws -> Int {
+    var modifiedPageCount = 0
+    // Make sure all properties are up to date
+    batchUpdatePageProperties()
+    for (pageIdentifier, contents) in pageContentsCache where contents.dirty {
+      let newTextSnippet = archive.insert(contents.text)
+      // Because we updated all page properties, safe to force-unwrap
+      let newPropertiesSnippet = archive.insert(try contents.pageProperties!.makeSnippet())
+      pageContentsCache[pageIdentifier]?.dirty = false
+      modifiedPageCount += 1
+
+      // If there was already content for this page in the archive, delta-encode it.
+      guard let (existingPropertiesSnippet, existingProperties) = try? currentPageProperties(for: pageIdentifier) else {
+        pagePropertyDigests[pageIdentifier] = newPropertiesSnippet.sha1Digest
+        continue
+      }
+      // New content is the same as the old content
+      if newPropertiesSnippet.sha1Digest == existingPropertiesSnippet.sha1Digest {
+        continue
+      }
+      newPropertiesSnippet.encodeAsDiff(from: nil)
+      existingPropertiesSnippet.encodeAsDiff(from: newPropertiesSnippet)
+      guard let existingTextSnippet = archive.snippets[existingProperties.sha1Digest] else {
+        throw RetrievalError.noSuchPage(existingProperties.sha1Digest)
+      }
+      // Note the content can be the same but the properties can have different timestamps
+      // So, check and make sure we didn't wind up with identical content before delta encoding.
+      if newTextSnippet.sha1Digest != existingTextSnippet.sha1Digest {
+        newTextSnippet.encodeAsDiff(from: nil)
+        existingTextSnippet.encodeAsDiff(from: newTextSnippet)
+      }
+      pagePropertyDigests[pageIdentifier] = newPropertiesSnippet.sha1Digest
+    }
+    return modifiedPageCount
+  }
+
+  /// Gets the page properties for a page identifier.
+  ///
+  /// - note: We return both the snippet and the decoded properties so we have the option of adding delta encoding to the snippet
+  /// if we are updating the contents of the page.
+  ///
+  /// - parameter pageIdentifier: the page to retrieve properties for
+  /// - returns: A tuple containing the TextSnippet of serialized properties and the deserialized version of the properties
+  /// - throws: `RetrievalError.noSuchPage` if the page was not found in the archive.
   func currentPageProperties(
     for pageIdentifier: String
   ) throws -> (snippet: TextSnippet, properties: PageProperties) {
@@ -267,21 +370,6 @@ private extension NoteArchive {
     return (propertiesSnippet, try PageProperties(propertiesSnippet))
   }
 
-  /// Creates a new Version representing the current page manifest in the archive.
-  /// - parameter timestamp: The version timestamp.
-  /// - throws:
-  mutating func archivePageManifestVersion(timestamp: Date) throws {
-    let version = Version(timestamp: timestamp, digest: archivePageManifest())
-    if let existingVersion = pagePropertiesVersionHistory.last,
-      let oldManifestSnippet = archive.snippets[existingVersion.digest],
-      let newManifestSnippet = archive.snippets[version.digest] {
-      newManifestSnippet.encodeAsDiff(from: nil)
-      oldManifestSnippet.encodeAsDiff(from: newManifestSnippet)
-    }
-    pagePropertiesVersionHistory.append(version)
-    try archiveVersionHistory()
-  }
-
   /// Writes the version history array into the archive.
   /// - note: We keep only one copy of the version array in the archive
   /// - throws: `TextSnippetArchive.Error` if there is a problem creating the symbolic reference to the version snippet
@@ -308,6 +396,8 @@ private extension NoteArchive {
       .compactMap(Version.init)
   }
 
+  /// Writes the current `pagePropertyDigests` into the archive.
+  /// - returns: The sha1Digest of the snippet created to hold this version of the manifest.
   mutating func archivePageManifest() -> String {
     let manifest = pagePropertyDigests
       .map { "\($0.key) \($0.value)" }
@@ -317,6 +407,11 @@ private extension NoteArchive {
     return manifestSnippet.sha1Digest
   }
 
+  /// Loads a specific version of the page manifest from the archive.
+  /// - parameter archive: The archive to load from.
+  /// - parameter manifestIdentifier: The sha1Digest of a specific version of a manifest.
+  /// - returns: a dictionary mapping pageIdentifiers to sha1Digests of specific versions of pages.
+  /// - throws: `RetrievalError.noSuchPage` if the manifest is not in the archive.
   static func getPageManifest(
     from archive: TextSnippetArchive,
     manifestIdentifier: String
@@ -355,6 +450,32 @@ private extension NoteArchive {
   }
 }
 
+private extension TextSnippetArchive {
+  /// Given an array of pageContents, computes updated PageProperties for any that are stale.
+  /// - note: This is mutating because we have to update any challenge templates in the archive
+  /// - returns: An array where every entry has non-stale properties.
+  mutating func updatePageProperties(
+    in pageContents: [String: NoteArchive.PageContents],
+    parsingRules: ParsingRules
+  ) -> [String: NoteArchive.PageContents] {
+    pageContents.mapValues { pageContent in
+      guard pageContent.pagePropertiesStale else { return pageContent }
+      var pageContent = pageContent
+      let nodes = parsingRules.parse(pageContent.text)
+      let challengeTemplateKeys = nodes.archiveChallengeTemplates(to: &self)
+      pageContent.pageProperties = PageProperties(
+        sha1Digest: TextSnippet(pageContent.text).sha1Digest,
+        timestamp: pageContent.modifiedTimestamp,
+        hashtags: nodes.hashtags,
+        title: String(nodes.title.split(separator: "\n").first ?? ""),
+        cardTemplates: challengeTemplateKeys.map { $0.description }
+      )
+      pageContent.pagePropertiesStale = false
+      return pageContent
+    }
+  }
+}
+
 private extension Date {
   /// True if the receiver and `other` are "close enough"
   func closeEnough(to other: Date) -> Bool {

CommonplaceBookApp/NoteArchiveDocument+Importing.swift

@@ -31,6 +31,7 @@ public extension NoteArchiveDocument {
         )
         self.notifyObservers(of: self.noteArchive.pageProperties)
         self.invalidateSavedSnippets()
+        DDLogInfo("Finished importing file \(fileName)")
         if let completion = completion {
           DispatchQueue.main.async {
             completion()
@@ -97,8 +98,11 @@ private extension NoteArchiveDocument {
         }
       }
     }
-    group.notify(queue: .main) {
-      completion?()
+    group.notify(queue: noteArchiveQueue) {
+      self.noteArchive.batchUpdatePageProperties()
+      DispatchQueue.main.async {
+        completion?()
+      }
     }
   }
 }

CommonplaceBookApp/NoteArchiveDocument+TextEditViewController.swift

@@ -14,7 +14,7 @@ extension NoteArchiveDocument: TextEditViewControllerDelegate {
       let now = Date()
       do {
         let pageIdentifier = try noteArchiveQueue.sync {
-          try noteArchive.insertNote(markdown, contentChangeTime: now, versionTimestamp: now)
+          try noteArchive.insertNote(markdown, contentChangeTime: now)
         }
         viewController.pageIdentifier = pageIdentifier
         invalidateSavedSnippets()