Skip to content

Commit f51e400

Browse files
neonichuneonichu
authored
Cache mirrorIndex in DependencyMirrors (#6964)
This appears to be *very* expensive, so we should cache.
1 parent 3c23f33 commit f51e400

File tree

7 files changed

+63
-68
lines changed

7 files changed

+63
-68
lines changed

‎Sources/Commands/PackageTools/Config.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ extension SwiftPackageTool.Config {
8080
}
8181

8282
try config.applyLocal { mirrors in
83-
mirrors.set(mirror: mirror, for: original)
83+
try mirrors.set(mirror: mirror, for: original)
8484
}
8585
}
8686
}

‎Sources/PackageGraph/DependencyMirrors.swift

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import struct TSCBasic.StringError
2020
/// A collection of dependency mirrors.
2121
public final class DependencyMirrors: Equatable {
2222
private var index: [String: String]
23+
private var mirrorIndex: [PackageIdentity: PackageIdentity]
2324
private var reverseIndex: [String: [String]]
2425
private var visited: OrderedCollections.OrderedSet<String>
2526
private let lock = NSLock()
@@ -30,11 +31,13 @@ public final class DependencyMirrors: Equatable {
3031
}
3132
}
3233

33-
public init(_ mirrors: [String: String]) {
34+
public init(_ mirrors: [String: String] = [:]) throws {
3435
self.index = mirrors
3536
self.reverseIndex = [String: [String]]()
37+
self.mirrorIndex = [PackageIdentity: PackageIdentity]()
3638
for entry in mirrors {
3739
self.reverseIndex[entry.value, default: []].append(entry.key)
40+
self.mirrorIndex[try Self.parseLocation(entry.key)] = try Self.parseLocation(entry.value)
3841
}
3942
self.visited = .init()
4043
}
@@ -47,10 +50,11 @@ public final class DependencyMirrors: Equatable {
4750
/// - Parameters:
4851
/// - mirror: The mirror
4952
/// - for: The original
50-
public func set(mirror: String, for key: String) {
51-
self.lock.withLock {
53+
public func set(mirror: String, for key: String) throws {
54+
try self.lock.withLock {
5255
self.index[key] = mirror
5356
self.reverseIndex[mirror, default: []].append(key)
57+
self.mirrorIndex[try Self.parseLocation(key)] = try Self.parseLocation(mirror)
5458
}
5559
}
5660

@@ -63,9 +67,11 @@ public final class DependencyMirrors: Equatable {
6367
if let value = self.index[originalOrMirror] {
6468
self.index[originalOrMirror] = nil
6569
self.reverseIndex[value] = nil
70+
self.mirrorIndex[try Self.parseLocation(value)] = nil
6671
} else if let mirror = self.index.first(where: { $0.value == originalOrMirror }) {
6772
self.index[mirror.key] = nil
6873
self.reverseIndex[originalOrMirror] = nil
74+
self.mirrorIndex[try Self.parseLocation(originalOrMirror)] = nil
6975
} else {
7076
throw StringError("Mirror not found for '\(originalOrMirror)'")
7177
}
@@ -75,9 +81,9 @@ public final class DependencyMirrors: Equatable {
7581
/// Append the content of a different DependencyMirrors into this one
7682
/// - Parameters:
7783
/// - contentsOf: The DependencyMirrors to append from.
78-
public func append(contentsOf mirrors: DependencyMirrors) {
79-
mirrors.index.forEach {
80-
self.set(mirror: $0.value, for: $0.key)
84+
public func append(contentsOf mirrors: DependencyMirrors) throws {
85+
try mirrors.index.forEach {
86+
try self.set(mirror: $0.value, for: $0.key)
8187
}
8288
}
8389

@@ -153,15 +159,10 @@ public final class DependencyMirrors: Equatable {
153159
}
154160

155161
public func effectiveIdentity(for identity: PackageIdentity) throws -> PackageIdentity {
156-
// TODO: cache
157-
let mirrorIndex = try self.mapping.reduce(into: [PackageIdentity: PackageIdentity]()) { partial, item in
158-
try partial[parseLocation(item.key)] = parseLocation(item.value)
159-
}
160-
161162
return mirrorIndex[identity] ?? identity
162163
}
163164

164-
private func parseLocation(_ location: String) throws -> PackageIdentity {
165+
private static func parseLocation(_ location: String) throws -> PackageIdentity {
165166
if PackageIdentity.plain(location).isRegistry {
166167
return PackageIdentity.plain(location)
167168
} else if let path = try? AbsolutePath(validating: location) {
@@ -200,9 +201,3 @@ extension DependencyMirrors: Collection {
200201
}
201202
}
202203
}
203-
204-
extension DependencyMirrors: ExpressibleByDictionaryLiteral {
205-
public convenience init(dictionaryLiteral elements: (String, String)...) {
206-
self.init(Dictionary(elements, uniquingKeysWith: { first, _ in first }))
207-
}
208-
}

‎Sources/SPMTestSupport/MockWorkspace.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public final class MockWorkspace {
6868
self.packages = packages
6969
self.fingerprints = customFingerprints ?? MockPackageFingerprintStorage()
7070
self.signingEntities = customSigningEntities ?? MockPackageSigningEntityStorage()
71-
self.mirrors = customMirrors ?? DependencyMirrors()
71+
self.mirrors = try customMirrors ?? DependencyMirrors()
7272
self.identityResolver = DefaultIdentityResolver(
7373
locationMapper: self.mirrors.effective(for:),
7474
identityMapper: self.mirrors.effectiveIdentity(for:)

‎Sources/Workspace/Workspace+Configuration.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ extension Workspace.Configuration {
462462
.map { .init(path: $0, fileSystem: fileSystem, deleteWhenEmpty: false) }
463463
self.fileSystem = fileSystem
464464
// computes the initial mirrors
465-
self._mirrors = DependencyMirrors()
465+
self._mirrors = try DependencyMirrors()
466466
try self.computeMirrors()
467467
}
468468

@@ -492,13 +492,13 @@ extension Workspace.Configuration {
492492
// prefer local mirrors to shared ones
493493
let local = try self.localMirrors.get()
494494
if !local.isEmpty {
495-
self._mirrors.append(contentsOf: local)
495+
try self._mirrors.append(contentsOf: local)
496496
return
497497
}
498498

499499
// use shared if local was not found or empty
500500
if let shared = try self.sharedMirrors?.get(), !shared.isEmpty {
501-
self._mirrors.append(contentsOf: shared)
501+
try self._mirrors.append(contentsOf: shared)
502502
}
503503
}
504504
}
@@ -520,10 +520,10 @@ extension Workspace.Configuration {
520520
/// The mirrors in this configuration
521521
public func get() throws -> DependencyMirrors {
522522
guard self.fileSystem.exists(self.path) else {
523-
return DependencyMirrors()
523+
return try DependencyMirrors()
524524
}
525525
return try self.fileSystem.withLock(on: self.path.parentDirectory, type: .shared) {
526-
return DependencyMirrors(try Self.load(self.path, fileSystem: self.fileSystem))
526+
return try DependencyMirrors(try Self.load(self.path, fileSystem: self.fileSystem))
527527
}
528528
}
529529

@@ -534,8 +534,8 @@ extension Workspace.Configuration {
534534
try self.fileSystem.createDirectory(self.path.parentDirectory, recursive: true)
535535
}
536536
return try self.fileSystem.withLock(on: self.path.parentDirectory, type: .exclusive) {
537-
let mirrors = DependencyMirrors(try Self.load(self.path, fileSystem: self.fileSystem))
538-
var updatedMirrors = DependencyMirrors(mirrors.mapping)
537+
let mirrors = try DependencyMirrors(try Self.load(self.path, fileSystem: self.fileSystem))
538+
var updatedMirrors = try DependencyMirrors(mirrors.mapping)
539539
try handler(&updatedMirrors)
540540
if updatedMirrors != mirrors {
541541
try Self.save(

‎Tests/WorkspaceTests/MirrorsConfigurationTests.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ final class MirrorsConfigurationTests: XCTestCase {
7373
let mirrorURL = "https://github.com/mona/swift-argument-parser.git"
7474

7575
try config.apply{ mirrors in
76-
mirrors.set(mirror: mirrorURL, for: originalURL)
76+
try mirrors.set(mirror: mirrorURL, for: originalURL)
7777
}
7878
XCTAssertTrue(fs.exists(configFile))
7979

@@ -96,7 +96,7 @@ final class MirrorsConfigurationTests: XCTestCase {
9696
let mirrorURL = "https://github.com/mona/swift-argument-parser.git"
9797

9898
try config.apply{ mirrors in
99-
mirrors.set(mirror: mirrorURL, for: originalURL)
99+
try mirrors.set(mirror: mirrorURL, for: originalURL)
100100
}
101101
XCTAssertTrue(fs.exists(configFile))
102102

@@ -124,7 +124,7 @@ final class MirrorsConfigurationTests: XCTestCase {
124124
let mirror1URL = "https://github.com/mona/swift-argument-parser.git"
125125

126126
try config.applyShared { mirrors in
127-
mirrors.set(mirror: mirror1URL, for: original1URL)
127+
try mirrors.set(mirror: mirror1URL, for: original1URL)
128128
}
129129

130130
XCTAssertEqual(config.mirrors.count, 1)
@@ -137,7 +137,7 @@ final class MirrorsConfigurationTests: XCTestCase {
137137
let mirror2URL = "https://github.com/mona/swift-nio.git"
138138

139139
try config.applyLocal { mirrors in
140-
mirrors.set(mirror: mirror2URL, for: original2URL)
140+
try mirrors.set(mirror: mirror2URL, for: original2URL)
141141
}
142142

143143
XCTAssertEqual(config.mirrors.count, 1)

‎Tests/WorkspaceTests/PinsStoreTests.swift

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,9 @@ final class PinsStoreTests: XCTestCase {
348348
let bazURL = SourceControlURL("https://github.com/cool/baz.git")
349349
let bazIdentity = PackageIdentity(url: bazURL)
350350

351-
let mirrors = DependencyMirrors()
352-
mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL.absoluteString)
353-
mirrors.set(mirror: barMirroredURL.absoluteString, for: barURL.absoluteString)
351+
let mirrors = try DependencyMirrors()
352+
try mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL.absoluteString)
353+
try mirrors.set(mirror: barMirroredURL.absoluteString, for: barURL.absoluteString)
354354

355355
let fileSystem = InMemoryFileSystem()
356356
let pinsFile = AbsolutePath("/pins.txt")
@@ -398,11 +398,11 @@ final class PinsStoreTests: XCTestCase {
398398
let fooURL4 = SourceControlURL("https://github.com/old-corporate/foo.git")
399399
let fooMirroredURL = SourceControlURL("https://github.corporate.com/team/foo")
400400

401-
let mirrors = DependencyMirrors()
402-
mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL1.absoluteString)
403-
mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL2.absoluteString)
404-
mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL3.absoluteString)
405-
mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL4.absoluteString)
401+
let mirrors = try DependencyMirrors()
402+
try mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL1.absoluteString)
403+
try mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL2.absoluteString)
404+
try mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL3.absoluteString)
405+
try mirrors.set(mirror: fooMirroredURL.absoluteString, for: fooURL4.absoluteString)
406406

407407
let fileSystem = InMemoryFileSystem()
408408
let pinsFile = AbsolutePath("/pins.txt")
@@ -445,7 +445,7 @@ final class PinsStoreTests: XCTestCase {
445445
let mirroredURL = URL("https://github.corporate.com/team/foo")
446446

447447
do {
448-
let mirrors = DependencyMirrors([
448+
let mirrors = try DependencyMirrors([
449449
URL1.absoluteString: mirroredURL.absoluteString,
450450
URL2.absoluteString: mirroredURL.absoluteString,
451451
URL3.absoluteString: mirroredURL.absoluteString,
@@ -458,7 +458,7 @@ final class PinsStoreTests: XCTestCase {
458458
}
459459

460460
do {
461-
let mirrors = DependencyMirrors([
461+
let mirrors = try DependencyMirrors([
462462
URL1.absoluteString: mirroredURL.absoluteString,
463463
URL2.absoluteString: mirroredURL.absoluteString,
464464
URL3.absoluteString: mirroredURL.absoluteString,
@@ -471,7 +471,7 @@ final class PinsStoreTests: XCTestCase {
471471
}
472472

473473
do {
474-
let mirrors = DependencyMirrors([
474+
let mirrors = try DependencyMirrors([
475475
URL1.absoluteString: mirroredURL.absoluteString,
476476
URL2.absoluteString: mirroredURL.absoluteString,
477477
URL3.absoluteString: mirroredURL.absoluteString,
@@ -485,7 +485,7 @@ final class PinsStoreTests: XCTestCase {
485485
}
486486

487487
do {
488-
let mirrors = DependencyMirrors([
488+
let mirrors = try DependencyMirrors([
489489
URL1.absoluteString: mirroredURL.absoluteString,
490490
URL2.absoluteString: mirroredURL.absoluteString,
491491
URL3.absoluteString: mirroredURL.absoluteString,
@@ -499,7 +499,7 @@ final class PinsStoreTests: XCTestCase {
499499
}
500500

501501
do {
502-
let mirrors = DependencyMirrors([
502+
let mirrors = try DependencyMirrors([
503503
URL1.absoluteString: mirroredURL.absoluteString,
504504
URL2.absoluteString: mirroredURL.absoluteString,
505505
URL3.absoluteString: mirroredURL.absoluteString,

‎Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4279,12 +4279,12 @@ final class WorkspaceTests: XCTestCase {
42794279
let sandbox = AbsolutePath("/tmp/ws/")
42804280
let fs = InMemoryFileSystem()
42814281

4282-
let mirrors = DependencyMirrors()
4283-
mirrors.set(
4282+
let mirrors = try DependencyMirrors()
4283+
try mirrors.set(
42844284
mirror: sandbox.appending(components: "pkgs", "BarMirror").pathString,
42854285
for: sandbox.appending(components: "pkgs", "Bar").pathString
42864286
)
4287-
mirrors.set(
4287+
try mirrors.set(
42884288
mirror: sandbox.appending(components: "pkgs", "BazMirror").pathString,
42894289
for: sandbox.appending(components: "pkgs", "Baz").pathString
42904290
)
@@ -4371,12 +4371,12 @@ final class WorkspaceTests: XCTestCase {
43714371
let sandbox = AbsolutePath("/tmp/ws/")
43724372
let fs = InMemoryFileSystem()
43734373

4374-
let mirrors = DependencyMirrors()
4375-
mirrors.set(
4374+
let mirrors = try DependencyMirrors()
4375+
try mirrors.set(
43764376
mirror: sandbox.appending(components: "pkgs", "BarMirror").pathString,
43774377
for: sandbox.appending(components: "pkgs", "Bar").pathString
43784378
)
4379-
mirrors.set(
4379+
try mirrors.set(
43804380
mirror: sandbox.appending(components: "pkgs", "BarMirror").pathString,
43814381
for: sandbox.appending(components: "pkgs", "Baz").pathString
43824382
)
@@ -4474,9 +4474,9 @@ final class WorkspaceTests: XCTestCase {
44744474
let sandbox = AbsolutePath("/tmp/ws/")
44754475
let fs = InMemoryFileSystem()
44764476

4477-
let mirrors = DependencyMirrors()
4478-
mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "https://scm.com/org/bar")
4479-
mirrors.set(mirror: "https://scm.com/org/baz-mirror", for: "https://scm.com/org/baz")
4477+
let mirrors = try DependencyMirrors()
4478+
try mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "https://scm.com/org/bar")
4479+
try mirrors.set(mirror: "https://scm.com/org/baz-mirror", for: "https://scm.com/org/baz")
44804480

44814481
let workspace = try MockWorkspace(
44824482
sandbox: sandbox,
@@ -4563,9 +4563,9 @@ final class WorkspaceTests: XCTestCase {
45634563
let sandbox = AbsolutePath("/tmp/ws/")
45644564
let fs = InMemoryFileSystem()
45654565

4566-
let mirrors = DependencyMirrors()
4567-
mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "https://scm.com/org/bar")
4568-
mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "https://scm.com/org/baz")
4566+
let mirrors = try DependencyMirrors()
4567+
try mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "https://scm.com/org/bar")
4568+
try mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "https://scm.com/org/baz")
45694569

45704570
let workspace = try MockWorkspace(
45714571
sandbox: sandbox,
@@ -4668,8 +4668,8 @@ final class WorkspaceTests: XCTestCase {
46684668
let sandbox = AbsolutePath("/tmp/ws/")
46694669
let fs = InMemoryFileSystem()
46704670

4671-
let mirrors = DependencyMirrors()
4672-
mirrors.set(mirror: "org.bar-mirror", for: "https://scm.com/org/bar")
4671+
let mirrors = try DependencyMirrors()
4672+
try mirrors.set(mirror: "org.bar-mirror", for: "https://scm.com/org/bar")
46734673

46744674
let workspace = try MockWorkspace(
46754675
sandbox: sandbox,
@@ -4738,8 +4738,8 @@ final class WorkspaceTests: XCTestCase {
47384738
let sandbox = AbsolutePath("/tmp/ws/")
47394739
let fs = InMemoryFileSystem()
47404740

4741-
let mirrors = DependencyMirrors()
4742-
mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "org.bar")
4741+
let mirrors = try DependencyMirrors()
4742+
try mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "org.bar")
47434743

47444744
let workspace = try MockWorkspace(
47454745
sandbox: sandbox,
@@ -14366,8 +14366,8 @@ final class WorkspaceTests: XCTestCase {
1436614366
}
1436714367

1436814368
func testSigningEntityVerification_MirroredSignedCorrectly() throws {
14369-
let mirrors = DependencyMirrors()
14370-
mirrors.set(mirror: "ecorp.bar", for: "org.bar")
14369+
let mirrors = try DependencyMirrors()
14370+
try mirrors.set(mirror: "ecorp.bar", for: "org.bar")
1437114371

1437214372
let actualMetadata = RegistryReleaseMetadata.createWithSigningEntity(.recognized(
1437314373
type: "adp",
@@ -14390,8 +14390,8 @@ final class WorkspaceTests: XCTestCase {
1439014390
}
1439114391

1439214392
func testSigningEntityVerification_MirrorSignedIncorrectly() throws {
14393-
let mirrors = DependencyMirrors()
14394-
mirrors.set(mirror: "ecorp.bar", for: "org.bar")
14393+
let mirrors = try DependencyMirrors()
14394+
try mirrors.set(mirror: "ecorp.bar", for: "org.bar")
1439514395

1439614396
let actualMetadata = RegistryReleaseMetadata.createWithSigningEntity(.recognized(
1439714397
type: "adp",
@@ -14422,8 +14422,8 @@ final class WorkspaceTests: XCTestCase {
1442214422
}
1442314423

1442414424
func testSigningEntityVerification_MirroredUnsigned() throws {
14425-
let mirrors = DependencyMirrors()
14426-
mirrors.set(mirror: "ecorp.bar", for: "org.bar")
14425+
let mirrors = try DependencyMirrors()
14426+
try mirrors.set(mirror: "ecorp.bar", for: "org.bar")
1442714427

1442814428
let expectedSigningEntity: RegistryReleaseMetadata.SigningEntity = .recognized(
1442914429
type: "adp",
@@ -14447,8 +14447,8 @@ final class WorkspaceTests: XCTestCase {
1444714447
}
1444814448

1444914449
func testSigningEntityVerification_MirroredToSCM() throws {
14450-
let mirrors = DependencyMirrors()
14451-
mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "org.bar")
14450+
let mirrors = try DependencyMirrors()
14451+
try mirrors.set(mirror: "https://scm.com/org/bar-mirror", for: "org.bar")
1445214452

1445314453
let expectedSigningEntity: RegistryReleaseMetadata.SigningEntity = .recognized(
1445414454
type: "adp",

0 commit comments

Comments
 (0)