implement naive search (#3044)

tomerd · web-flow · commit e8b552b9ec7d · 2020-11-13T16:00:43.000-08:00
motivation: continue to implement package collections business logic

changes:
* implement naive in-memory search for packages and targets
* performance work on "list" by making decoding in parallel and in-memory cache (will be improved)
* enbale performance tests
diff --git a/Sources/PackageCollections/Model/Package.swift b/Sources/PackageCollections/Model/Package.swift
@@ -121,7 +121,7 @@ extension PackageCollectionsModel.Package {
 
 extension PackageCollectionsModel {
     /// A representation of package target
-    public struct PackageTarget: Equatable, Codable {
+    public struct PackageTarget: Equatable, Hashable, Codable {
         /// The target name
         public let name: String
 
diff --git a/Sources/PackageCollections/Model/Search.swift b/Sources/PackageCollections/Model/Search.swift
@@ -34,6 +34,6 @@ extension PackageCollectionsModel {
     /// A representation of target in search result
     public struct TargetSearchResult {
         /// Result items of the search
-        public let items: TargetListResult
+        public let items: [TargetListItem]
     }
 }
diff --git a/Sources/PackageCollections/Storage/PackageCollectionsProfileStorage.swift b/Sources/PackageCollections/Storage/PackageCollectionsProfileStorage.swift
@@ -85,8 +85,8 @@ struct FilePackageCollectionsProfileStorage: PackageCollectionsProfileStorage {
     let fileSystem: FileSystem
     let path: AbsolutePath
 
-    private let jsonEncoder: JSONEncoder
-    private let jsonDecoder: JSONDecoder
+    private let encoder: JSONEncoder
+    private let decoder: JSONDecoder
 
     private let queue = DispatchQueue(label: "org.swift.swiftpm.FilePackageCollectionsProfileStorage")
 
@@ -96,13 +96,13 @@ struct FilePackageCollectionsProfileStorage: PackageCollectionsProfileStorage {
         let name = "collections"
         self.path = path ?? fileSystem.dotSwiftPM.appending(component: "\(name).json")
 
-        self.jsonEncoder = JSONEncoder()
-        self.jsonEncoder.outputFormatting = .prettyPrinted
+        self.encoder = JSONEncoder()
+        self.encoder.outputFormatting = .prettyPrinted
         if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) {
-            self.jsonEncoder.outputFormatting.insert(.sortedKeys)
-            self.jsonEncoder.outputFormatting.insert(.withoutEscapingSlashes)
+            self.encoder.outputFormatting.insert(.sortedKeys)
+            self.encoder.outputFormatting.insert(.withoutEscapingSlashes)
         }
-        self.jsonDecoder = JSONDecoder()
+        self.decoder = JSONDecoder()
     }
 
     func listProfiles(callback: @escaping (Result<[PackageCollectionsModel.Profile], Error>) -> Void) {
@@ -224,7 +224,7 @@ struct FilePackageCollectionsProfileStorage: PackageCollectionsProfileStorage {
         guard buffer.count > 0 else {
             return .init()
         }
-        let container = try jsonDecoder.decode(Model.Container.self, from: Data(buffer))
+        let container = try decoder.decode(Model.Container.self, from: Data(buffer))
         return try container.profiles()
     }
 
@@ -233,7 +233,7 @@ struct FilePackageCollectionsProfileStorage: PackageCollectionsProfileStorage {
             try self.fileSystem.createDirectory(self.path.parentDirectory, recursive: true)
         }
         let container = Model.Container(profiles)
-        let buffer = try jsonEncoder.encode(container)
+        let buffer = try encoder.encode(container)
         try self.fileSystem.writeFileContents(self.path, bytes: ByteString(buffer))
     }
 
diff --git a/Sources/PackageCollections/Storage/PackageCollectionsStorage.swift b/Sources/PackageCollections/Storage/PackageCollectionsStorage.swift
@@ -94,15 +94,18 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
     let fileSystem: FileSystem
     let location: SQLite.Location
 
-    private let jsonEncoder: JSONEncoder
-    private let jsonDecoder: JSONDecoder
+    private let encoder: JSONEncoder
+    private let decoder: JSONDecoder
 
     // for concurrent for DB access
     private let queue = DispatchQueue(label: "org.swift.swiftpm.SQLitePackageCollectionsStorage", attributes: .concurrent)
 
     private var state = State.idle
     private let stateLock = Lock()
 
+    private var cache = [PackageCollectionsModel.CollectionIdentifier: PackageCollectionsModel.Collection]()
+    private let cacheLock = Lock()
+
     init(location: SQLite.Location? = nil) {
         self.location = location ?? .path(localFileSystem.swiftPMCacheDirectory.appending(components: "package-collection.db"))
         switch self.location {
@@ -111,8 +114,8 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
         case .memory:
             self.fileSystem = InMemoryFileSystem()
         }
-        self.jsonEncoder = JSONEncoder()
-        self.jsonDecoder = JSONDecoder()
+        self.encoder = JSONEncoder()
+        self.decoder = JSONDecoder()
     }
 
     convenience init(path: AbsolutePath) {
@@ -138,9 +141,10 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
              callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
         self.queue.async {
             do {
+                // write to db
                 let query = "INSERT OR IGNORE INTO PACKAGES_COLLECTIONS VALUES (?, ?);"
                 try self.executeStatement(query) { statement -> Void in
-                    let data = try self.jsonEncoder.encode(collection)
+                    let data = try self.encoder.encode(collection)
 
                     let bindings: [SQLite.SQLiteValue] = [
                         .string(collection.identifier.databaseKey()),
@@ -149,6 +153,10 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                     try statement.bind(bindings)
                     try statement.step()
                 }
+                // write to cache
+                self.cacheLock.withLock {
+                    self.cache[collection.identifier] = collection
+                }
                 callback(.success(collection))
             } catch {
                 callback(.failure(error))
@@ -160,6 +168,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                 callback: @escaping (Result<Void, Error>) -> Void) {
         self.queue.async {
             do {
+                // write to db
                 let query = "DELETE FROM PACKAGES_COLLECTIONS WHERE key == ?;"
                 try self.executeStatement(query) { statement -> Void in
                     let bindings: [SQLite.SQLiteValue] = [
@@ -168,6 +177,10 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                     try statement.bind(bindings)
                     try statement.step()
                 }
+                // write to cache
+                self.cacheLock.withLock {
+                    self.cache[identifier] = nil
+                }
                 callback(.success(()))
             } catch {
                 callback(.failure(error))
@@ -177,6 +190,12 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
 
     func get(identifier: PackageCollectionsModel.CollectionIdentifier,
              callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
+        // try read to cache
+        if let collection = (self.cacheLock.withLock { self.cache[identifier] }) {
+            return callback(.success(collection))
+        }
+
+        // go to db if not found
         self.queue.async {
             do {
                 let query = "SELECT value FROM PACKAGES_COLLECTIONS WHERE key == ? LIMIT 1;"
@@ -188,7 +207,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                         throw NotFoundError("\(identifier)")
                     }
 
-                    let collection = try self.jsonDecoder.decode(PackageCollectionsModel.Collection.self, from: data)
+                    let collection = try self.decoder.decode(PackageCollectionsModel.Collection.self, from: data)
                     return collection
                 }
                 callback(.success(collection))
@@ -200,11 +219,21 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
 
     func list(identifiers: [PackageCollectionsModel.CollectionIdentifier]? = nil,
               callback: @escaping (Result<[PackageCollectionsModel.Collection], Error>) -> Void) {
+        // try read to cache
+        let cached = self.cacheLock.withLock {
+            identifiers?.compactMap { identifier in
+                self.cache[identifier]
+            }
+        }
+        if let cached = cached, cached.count > 0, cached.count == identifiers?.count {
+            return callback(.success(cached))
+        }
+
+        // go to db if not found
         self.queue.async {
             do {
                 var blobs = [Data]()
                 if let identifiers = identifiers {
-                    // TODO: consider running these in parallel
                     var index = 0
                     while index < identifiers.count {
                         let slice = identifiers[index ..< min(index + Self.batchSize, identifiers.count)]
@@ -226,25 +255,86 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
                     }
                 }
 
-                // TODO: consider some diagnostics / warning for invalid data
-                let collections = blobs.compactMap { data -> PackageCollectionsModel.Collection? in
-                    try? self.jsonDecoder.decode(PackageCollectionsModel.Collection.self, from: data)
+                // decoding is a performance bottleneck (10+s for 1000 collections)
+                // workaround is to decode in parallel if list is large enough to justify it
+                var collections: [PackageCollectionsModel.Collection]
+                if blobs.count < 50 {
+                    // TODO: consider some diagnostics / warning for invalid data
+                    collections = blobs.compactMap { data -> PackageCollectionsModel.Collection? in
+                        try? self.decoder.decode(PackageCollectionsModel.Collection.self, from: data)
+                    }
+                } else {
+                    let lock = Lock()
+                    let sync = DispatchGroup()
+                    collections = [PackageCollectionsModel.Collection]()
+                    blobs.forEach { data in
+                        sync.enter()
+                        self.queue.async {
+                            defer { sync.leave() }
+                            if let collection = try? self.decoder.decode(PackageCollectionsModel.Collection.self, from: data) {
+                                lock.withLock {
+                                    collections.append(collection)
+                                }
+                            }
+                        }
+                    }
+                    sync.wait()
                 }
+
                 callback(.success(collections))
             } catch {
                 callback(.failure(error))
             }
         }
     }
 
-    // FIXME: implement this
+    // TODO: this is PoC for search, need a more performant version of this
     func searchPackages(identifiers: [PackageCollectionsModel.CollectionIdentifier]? = nil,
                         query: String,
                         callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult, Error>) -> Void) {
-        fatalError("not implemented")
+        let queryString = query.lowercased()
+
+        self.list(identifiers: identifiers) { result in
+            switch result {
+            case .failure(let error):
+                callback(.failure(error))
+            case .success(let collections):
+                let collectionsPackages = collections.reduce([PackageCollectionsModel.CollectionIdentifier: [PackageCollectionsModel.Collection.Package]]()) { partial, collection in
+                    var map = partial
+                    map[collection.identifier] = collection.packages.filter { package in
+                        if package.repository.url.lowercased().contains(queryString) { return true }
+                        if let summary = package.summary, summary.lowercased().contains(queryString) { return true }
+                        return package.versions.contains(where: { version in
+                            if version.packageName.lowercased().contains(queryString) { return true }
+                            if version.products.contains(where: { $0.name.lowercased().contains(queryString) }) { return true }
+                            return version.targets.contains(where: { $0.name.lowercased().contains(queryString) })
+                        })
+                    }
+                    return map
+                }
+
+                // compose result :p
+
+                var packageCollections = [PackageReference: (package: PackageCollectionsModel.Collection.Package, collections: Set<PackageCollectionsModel.CollectionIdentifier>)]()
+                collectionsPackages.forEach { collectionIdentifier, packages in
+                    packages.forEach { package in
+                        // Avoid copy-on-write: remove entry from dictionary before mutating
+                        var entry = packageCollections.removeValue(forKey: package.reference) ?? (package, .init())
+                        entry.collections.insert(collectionIdentifier)
+                        packageCollections[package.reference] = entry
+                    }
+                }
+
+                let result = PackageCollectionsModel.PackageSearchResult(items: packageCollections.map { entry in
+                    .init(package: entry.value.package, collections: Array(entry.value.collections))
+                })
+
+                callback(.success(result))
+            }
+        }
     }
 
-    // FIXME: this is PoC for search, need a more performant version of this
+    // TODO: this is PoC for search, need a more performant version of this
     func findPackage(identifier: PackageReference.PackageIdentity,
                      collectionIdentifiers: [PackageCollectionsModel.CollectionIdentifier]?,
                      callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void) {
@@ -269,14 +359,89 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
         }
     }
 
-    // FIXME: implement this
+    // TODO: this is PoC for search, need a more performant version of this
     func searchTargets(identifiers: [PackageCollectionsModel.CollectionIdentifier]? = nil,
                        query: String,
                        type: PackageCollectionsModel.TargetSearchType,
                        callback: @escaping (Result<PackageCollectionsModel.TargetSearchResult, Error>) -> Void) {
-        fatalError("not implemented")
+        let query = query.lowercased()
+
+        self.list(identifiers: identifiers) { result in
+            switch result {
+            case .failure(let error):
+                callback(.failure(error))
+            case .success(let collections):
+                let collectionsPackages = collections.reduce([PackageCollectionsModel.CollectionIdentifier: [(target: PackageCollectionsModel.PackageTarget, package: PackageCollectionsModel.Collection.Package)]]()) { partial, collection in
+                    var map = partial
+                    collection.packages.forEach { package in
+                        package.versions.forEach { version in
+                            version.targets.forEach { target in
+                                let match: Bool
+                                switch type {
+                                case .exactMatch:
+                                    match = target.name.lowercased() == query
+                                case .prefix:
+                                    match = target.name.lowercased().hasPrefix(query)
+                                }
+                                if match {
+                                    // Avoid copy-on-write: remove entry from dictionary before mutating
+                                    var entry = map.removeValue(forKey: collection.identifier) ?? .init()
+                                    entry.append((target, package))
+                                    map[collection.identifier] = entry
+                                }
+                            }
+                        }
+                    }
+                    return map
+                }
+
+                // compose result :p
+
+                var packageCollections = [PackageReference: (package: PackageCollectionsModel.Collection.Package, collections: Set<PackageCollectionsModel.CollectionIdentifier>)]()
+                var targetsPackages = [PackageCollectionsModel.PackageTarget: Set<PackageReference>]()
+
+                collectionsPackages.forEach { collectionIdentifier, packagesAndTargets in
+                    packagesAndTargets.forEach { item in
+                        // Avoid copy-on-write: remove entry from dictionary before mutating
+                        var packageCollectionsEntry = packageCollections.removeValue(forKey: item.package.reference) ?? (item.package, .init())
+                        packageCollectionsEntry.collections.insert(collectionIdentifier)
+                        packageCollections[item.package.reference] = packageCollectionsEntry
+
+                        // Avoid copy-on-write: remove entry from dictionary before mutating
+                        var targetsPackagesEntry = targetsPackages.removeValue(forKey: item.target) ?? .init()
+                        targetsPackagesEntry.insert(item.package.reference)
+                        targetsPackages[item.target] = targetsPackagesEntry
+                    }
+                }
+
+                let result = PackageCollectionsModel.TargetSearchResult(items: targetsPackages.map { target, packages in
+                    let targetsPackages = packages
+                        .compactMap { packageCollections[$0] }
+                        .map { pair -> PackageCollectionsModel.TargetListItem.Package in
+                            let versions = pair.package.versions.map { PackageCollectionsModel.TargetListItem.Package.Version(version: $0.version, packageName: $0.packageName) }
+                            return PackageCollectionsModel.TargetListItem.Package(repository: pair.package.repository,
+                                                                                  description: pair.package.summary,
+                                                                                  versions: versions,
+                                                                                  collections: Array(pair.collections))
+                        }
+
+                    return PackageCollectionsModel.TargetListItem(target: target, packages: targetsPackages)
+                })
+
+                callback(.success(result))
+            }
+        }
+    }
+
+    // for testing
+    internal func resetCache() {
+        self.cacheLock.withLock {
+            self.cache = [:]
+        }
     }
 
+    // MARK: -  Private
+
     private func createSchemaIfNecessary(db: SQLite) throws {
         let table = """
             CREATE TABLE IF NOT EXISTS PACKAGES_COLLECTIONS (
diff --git a/Tests/PackageCollectionsTests/PackageCollectionsStorageTests.swift b/Tests/PackageCollectionsTests/PackageCollectionsStorageTests.swift
@@ -89,6 +89,7 @@ class PackageCollectionsStorageTests: XCTestCase {
             XCTAssertTrue(storage.fileSystem.exists(storagePath), "expected file to exist at \(storagePath)")
 
             try storage.fileSystem.removeFileTree(storagePath)
+            storage.resetCache()
 
             XCTAssertThrowsError(try tsc_await { callback in storage.get(identifier: mockCollections.first!.identifier, callback: callback) }, "expected error", { error in
                 XCTAssert(error is NotFoundError, "Expected NotFoundError")
@@ -123,6 +124,7 @@ class PackageCollectionsStorageTests: XCTestCase {
             }
 
             try storage.close()
+            storage.resetCache()
 
             XCTAssertTrue(storage.fileSystem.exists(storagePath), "expected file to exist at \(path)")
             try storage.fileSystem.writeFileContents(storagePath, bytes: ByteString("blah".utf8))
diff --git a/Tests/PackageCollectionsTests/PackageCollectionsTests.swift b/Tests/PackageCollectionsTests/PackageCollectionsTests.swift
diff --git a/Tests/PackageCollectionsTests/Utility.swift b/Tests/PackageCollectionsTests/Utility.swift

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,6 @@ extension PackageCollectionsModel {`
`34`	`34`	`/// A representation of target in search result`
`35`	`35`	`public struct TargetSearchResult {`
`36`	`36`	`/// Result items of the search`
`37`		`- public let items: TargetListResult`
	`37`	`+ public let items: [TargetListItem]`
`38`	`38`	`}`
`39`	`39`	`}`