Skip to content

[Collections] Use persistent cache for GitHub package metadata #3441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 28, 2021

Conversation

yim-lee
Copy link
Contributor

@yim-lee yim-lee commented Apr 24, 2021

Motivation:
Currently we use transient in-memory cache for storing GitHub package metadata.

Modifications:

  • Add generic SQLiteBackedCache in Basics
  • Change ManifestLoader to use SQLiteBackedCache
  • Change GitHubPackageMetadataProvider to use SQLiteBackedCache
  • Adjust tests

@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 24, 2021

@swift-ci please smoke test

import TSCUtility

/// SQLite backed persistent cache.
public final class SQLiteBackedCache<Value: Codable>: Closable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is formerly SQLiteManifestCache

}
}

public func put(key: Key, value: Value, replace: Bool = false) throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add option to replace or ignore

if let cache = self.cache {
cache[reference] = (model, DispatchTime.now())

if cache.count > self.configuration.cacheSize {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the max size feature in SQLite cache, we don't need to manually delete cache entries.

@@ -585,23 +584,23 @@ public final class ManifestLoader: ManifestLoaderProtocol {
}

fileprivate func parseAndCacheManifest(key: ManifestCacheKey, diagnostics: DiagnosticsEngine?) -> ManifestParseResult {
let cache = self.databaseCacheDir.map { cacheDir -> SQLiteManifestCache in
let cache = self.databaseCacheDir.map { cacheDir -> SQLiteBackedCache<ManifestParseResult> in
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change in this file. The rest is formatting / refactoring.

import TSCUtility
import XCTest

final class SQLiteBackedCacheTests: XCTestCase {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied from manifest cache tests.

@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 24, 2021

@swift-ci please smoke test

1 similar comment
@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 24, 2021

@swift-ci please smoke test

Copy link
Contributor

@neonichu neonichu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👍

throw error
}
self.diagnosticsEngine?.emit(.warning("truncating \(self.name) cache database since it reached max size of \(self.configuration.maxSizeInBytes ?? 0) bytes"))
try self.executeStatement("DELETE FROM \(self.name);") { statement -> Void in
Copy link
Contributor

@tomerd tomerd Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as future idea, we can evolve configuration.truncateWhenFull to determine more fine grained rules

yim-lee added 5 commits April 26, 2021 14:28
Motivation:
Currently we use transient in-memory cache for storing GitHub package metadata.

Modifications:
- Add generic `SQLiteBackedCache` in Basics
- Change `ManifestLoader` to use `SQLiteBackedCache`
- Change `GitHubPackageMetadataProvider` to use `SQLiteBackedCache`
- Adjust tests
@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 27, 2021

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 27, 2021

@swift-ci please smoke test macOS

@tomerd
Copy link
Contributor

tomerd commented Apr 27, 2021

@swift-ci please smoke test macOS

@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 27, 2021

https://ci.swift.org/job/swift-package-manager-PR-macos-smoke-test/3140/console

16:10:29 Test Suite 'PackageDescription4_2LoadingTests' started at 2021-04-27 14:02:53.396
16:10:29 Test Case '-[PackageLoadingTests.PackageDescription4_2LoadingTests testConcurrencyNoWarmUp]' started.
16:10:29 
/Users/buildnode/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Tests/PackageLoadingTests/PD4_2LoadingTests.swift:868: error: -[PackageLoadingTests.PackageDescription4_2LoadingTests testConcurrencyNoWarmUp] : failed - timeout
16:10:29 /Users/buildnode/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Tests/PackageLoadingTests/PD4_2LoadingTests.swift:871: error: -[PackageLoadingTests.PackageDescription4_2LoadingTests testConcurrencyNoWarmUp] : XCTAssertEqual failed: ("24") is not equal to ("1000")
16:10:29 Test Case '-[PackageLoadingTests.PackageDescription4_2LoadingTests testConcurrencyNoWarmUp]' failed (669.931 seconds).
16:10:29 Test Suite 'PackageDescription4_2LoadingTests' failed at 2021-04-27 14:14:03.328.
16:10:29 	 Executed 1 test, with 2 failures (0 unexpected) in 669.931 (669.931) seconds
16:10:29 Test Suite 'SwiftPMPackageTests.xctest' failed at 2021-04-27 14:14:03.328.
16:10:29 	 Executed 1 test, with 2 failures (0 unexpected) in 669.931 (669.932) seconds
16:10:29 Test Suite 'Selected tests' failed at 2021-04-27 14:14:03.328.
16:10:29 	 Executed 1 test, with 2 failures (0 unexpected) in 669.931 (669.933) seconds

@yim-lee
Copy link
Contributor Author

yim-lee commented Apr 27, 2021

@swift-ci please smoke test macOS

@yim-lee yim-lee merged commit 957707b into swiftlang:main Apr 28, 2021
yim-lee added a commit to yim-lee/swift-package-manager that referenced this pull request Apr 28, 2021
…lang#3441)

* [Collections] Use persistent cache for GitHub package metadata

Motivation:
Currently we use transient in-memory cache for storing GitHub package metadata.

Modifications:
- Add generic `SQLiteBackedCache` in Basics
- Change `ManifestLoader` to use `SQLiteBackedCache`
- Change `GitHubPackageMetadataProvider` to use `SQLiteBackedCache`
- Adjust tests

* Undo swiftformat changes

* Fix CMake build

* Rename name to tableName

* Document 'tableName'
yim-lee added a commit that referenced this pull request Apr 28, 2021
#3453)

* [Collections] Use persistent cache for GitHub package metadata

Motivation:
Currently we use transient in-memory cache for storing GitHub package metadata.

Modifications:
- Add generic `SQLiteBackedCache` in Basics
- Change `ManifestLoader` to use `SQLiteBackedCache`
- Change `GitHubPackageMetadataProvider` to use `SQLiteBackedCache`
- Adjust tests

* Undo swiftformat changes

* Fix CMake build

* Rename name to tableName

* Document 'tableName'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants