Skip to content

Commit 49c22f9

Browse files
authored
Merge pull request #797 from owenv/owenv/description-refcounting
Add API to allow clients to manage build description lifecycle
2 parents 8f33527 + 8f622e1 commit 49c22f9

15 files changed

+212
-35
lines changed

Sources/SWBBuildService/BuildDescriptionMessages.swift

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ fileprivate extension Request {
5454
message.buildDescriptionID,
5555
request: buildRequest,
5656
buildRequestContext: buildRequestContext,
57-
workspaceContext: workspaceContext
57+
workspaceContext: workspaceContext,
58+
retain: false
5859
), clientDelegate: clientDelegate, constructionDelegate: operation
5960
)?.buildDescription
6061

@@ -220,3 +221,11 @@ struct IndexBuildSettingsMsg: MessageHandler {
220221
return IndexBuildSettingsResponse(compilerArguments: compilerArguments)
221222
}
222223
}
224+
225+
struct ReleaseBuildDescriptionMsg: MessageHandler {
226+
func handle(request: Request, message: ReleaseBuildDescriptionRequest) async throws -> VoidResponse {
227+
let session = try request.session(for: message)
228+
session.buildDescriptionManager.releaseBuildDescription(id: message.buildDescriptionID)
229+
return VoidResponse()
230+
}
231+
}

Sources/SWBBuildService/BuildOperationMessages.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ final class ActiveBuild: ActiveBuildOperation {
165165
/// Whether this operation is intended only for creating and reporting the build description.
166166
let onlyCreatesBuildDescription: Bool
167167

168+
/// Whether this operation should retain the build description it uses.
169+
let retainBuildDescription: Bool
170+
168171
/// The current state of the build.
169172
var state: State
170173

@@ -194,6 +197,7 @@ final class ActiveBuild: ActiveBuildOperation {
194197
self.id = request.buildService.nextBuildOperationID()
195198

196199
self.onlyCreatesBuildDescription = message.onlyCreateBuildDescription
200+
self.retainBuildDescription = message.retainBuildDescription == true
197201

198202
self.session = try request.session(for: message)
199203
guard let workspaceContext = session.workspaceContext else {
@@ -420,7 +424,7 @@ final class ActiveBuild: ActiveBuildOperation {
420424
let preparationDelegate = self.preparationProgressDelegate!
421425
let clientDelegate = ClientExchangeDelegate(request: self.request, session: self.session)
422426
// FIXME: We should have a channel for reporting errors here which don't make it look like there was an internal service error. E.g., if we fail to create or write the build description or manifest because of some error outside of our control, we should simply report that and not make it look like we might have a bug.
423-
let description = try await MacroNamespace.withExpressionInterningEnabled { try await self.session.buildDescriptionManager.getBuildDescription(planRequest, clientDelegate: clientDelegate, constructionDelegate: preparationDelegate) }
427+
let description = try await MacroNamespace.withExpressionInterningEnabled { try await self.session.buildDescriptionManager.getBuildDescription(planRequest, retained: retainBuildDescription, clientDelegate: clientDelegate, constructionDelegate: preparationDelegate) }
424428
return description
425429
} catch {
426430
self.abortBuild(error)
@@ -445,7 +449,7 @@ final class ActiveBuild: ActiveBuildOperation {
445449

446450
do {
447451
let clientDelegate = ClientExchangeDelegate(request: self.request, session: self.session)
448-
let descRequest = BuildDescriptionManager.BuildDescriptionRequest.cachedOnly(buildDescriptionID, request: self.buildRequest, buildRequestContext: self.buildRequestContext, workspaceContext: self.workspaceContext)
452+
let descRequest = BuildDescriptionManager.BuildDescriptionRequest.cachedOnly(buildDescriptionID, request: self.buildRequest, buildRequestContext: self.buildRequestContext, workspaceContext: self.workspaceContext, retain: self.retainBuildDescription)
449453
let retrievedBuildDescription = try await self.session.buildDescriptionManager.getNewOrCachedBuildDescription(descRequest, clientDelegate: clientDelegate, constructionDelegate: self.preparationProgressDelegate!)
450454
return retrievedBuildDescription?.buildDescription
451455
} catch {

Sources/SWBBuildService/DocumentationInfo.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ extension BuildDescriptionManager {
5151
// Get the complete build description.
5252
let buildDescription: BuildDescription
5353
do {
54-
if let retrievedBuildDescription = try await getBuildDescription(planRequest, clientDelegate: delegate.clientDelegate, constructionDelegate: delegate) {
54+
if let retrievedBuildDescription = try await getBuildDescription(planRequest, retained: false, clientDelegate: delegate.clientDelegate, constructionDelegate: delegate) {
5555
buildDescription = retrievedBuildDescription
5656
} else {
5757
// If we don't receive a build description it means we were cancelled.

Sources/SWBBuildService/LocalizationInfo.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ extension BuildDescriptionManager {
6868

6969
let buildDescription: BuildDescription
7070
do {
71-
if let retrievedBuildDescription = try await getNewOrCachedBuildDescription(.cachedOnly(descriptionID, request: buildRequest, buildRequestContext: buildRequestContext, workspaceContext: workspaceContext), clientDelegate: delegate.clientDelegate, constructionDelegate: delegate)?.buildDescription {
71+
if let retrievedBuildDescription = try await getNewOrCachedBuildDescription(.cachedOnly(descriptionID, request: buildRequest, buildRequestContext: buildRequestContext, workspaceContext: workspaceContext, retain: false), clientDelegate: delegate.clientDelegate, constructionDelegate: delegate)?.buildDescription {
7272
buildDescription = retrievedBuildDescription
7373
} else {
7474
// If we don't receive a build description it means we were cancelled.

Sources/SWBBuildService/Messages.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ fileprivate enum ResultOrErrorMessage<T> {
585585
fileprivate func getIndexBuildDescriptionFromID(buildDescriptionID: BuildDescriptionID, request: Request, session: Session, buildRequest: BuildRequest, buildRequestContext: BuildRequestContext, workspaceContext: WorkspaceContext, constructionDelegate: any BuildDescriptionConstructionDelegate) async -> ResultOrErrorMessage<BuildDescription> {
586586
let clientDelegate = ClientExchangeDelegate(request: request, session: session)
587587
do {
588-
let descRequest = BuildDescriptionManager.BuildDescriptionRequest.cachedOnly(buildDescriptionID, request: buildRequest, buildRequestContext: buildRequestContext, workspaceContext: workspaceContext)
588+
let descRequest = BuildDescriptionManager.BuildDescriptionRequest.cachedOnly(buildDescriptionID, request: buildRequest, buildRequestContext: buildRequestContext, workspaceContext: workspaceContext, retain: false)
589589
guard let retrievedBuildDescription = try await session.buildDescriptionManager.getNewOrCachedBuildDescription(descRequest, clientDelegate: clientDelegate, constructionDelegate: constructionDelegate) else {
590590
// If we don't receive a build description it means we were cancelled.
591591
return .failed(VoidResponse())
@@ -727,7 +727,7 @@ extension MessageHandler {
727727
let clientDelegate = ClientExchangeDelegate(request: request, session: session)
728728
let buildDescription: BuildDescription
729729
do {
730-
if let retrievedBuildDescription = try await session.buildDescriptionManager.getBuildDescription(planRequest, clientDelegate: clientDelegate, constructionDelegate: operation) {
730+
if let retrievedBuildDescription = try await session.buildDescriptionManager.getBuildDescription(planRequest, retained: false, clientDelegate: clientDelegate, constructionDelegate: operation) {
731731
buildDescription = retrievedBuildDescription
732732
} else {
733733
// If we don't receive a build description it means we were cancelled.
@@ -1626,6 +1626,7 @@ package struct ServiceMessageHandlers: ServiceExtension {
16261626
service.registerMessageHandler(BuildDescriptionConfiguredTargetsMsg.self)
16271627
service.registerMessageHandler(BuildDescriptionConfiguredTargetSourcesMsg.self)
16281628
service.registerMessageHandler(IndexBuildSettingsMsg.self)
1629+
service.registerMessageHandler(ReleaseBuildDescriptionMsg.self)
16291630

16301631
service.registerMessageHandler(MacroEvaluationMsg.self)
16311632
service.registerMessageHandler(AllExportedMacrosAndValuesMsg.self)

Sources/SWBBuildService/PreviewInfo.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ extension BuildDescriptionManager {
123123
do {
124124
if let retrievedBuildDescription = try await getBuildDescription(
125125
planRequest,
126+
retained: false,
126127
clientDelegate: delegate.clientDelegate,
127128
constructionDelegate: delegate
128129
) {

Sources/SWBProtocol/BuildDescriptionMessages.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,24 @@ public struct IndexBuildSettingsResponse: Message, SerializableCodable, Equatabl
213213
}
214214
}
215215

216+
public struct ReleaseBuildDescriptionRequest: SessionMessage, RequestMessage, SerializableCodable, Equatable {
217+
public typealias ResponseMessage = VoidResponse
218+
219+
public static let name = "RELEASE_BUILD_DESCRIPTION"
220+
221+
public var sessionHandle: String
222+
223+
public let buildDescriptionID: BuildDescriptionID
224+
225+
public init(
226+
sessionHandle: String,
227+
buildDescriptionID: BuildDescriptionID
228+
) {
229+
self.sessionHandle = sessionHandle
230+
self.buildDescriptionID = buildDescriptionID
231+
}
232+
}
233+
216234
// MARK: Registering messages
217235

218236
let buildDescriptionMessages: [any Message.Type] = [
@@ -222,4 +240,5 @@ let buildDescriptionMessages: [any Message.Type] = [
222240
BuildDescriptionConfiguredTargetSourcesResponse.self,
223241
IndexBuildSettingsRequest.self,
224242
IndexBuildSettingsResponse.self,
243+
ReleaseBuildDescriptionRequest.self,
225244
]

Sources/SWBProtocol/BuildOperationMessages.swift

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,19 +263,24 @@ public struct CreateBuildRequest: SessionChannelBuildMessage, RequestMessage, Se
263263
/// If true, the build operation will be completed after the build description is created and reported.
264264
public let onlyCreateBuildDescription: Bool
265265

266+
/// If true, the build description for this build wil be kept alive in memory, even if it would otherwise be evicted from caches.
267+
public let retainBuildDescription: Bool?
268+
269+
@available(*, deprecated, renamed: "init(sessionHandle:responseChannel:request:onlyCreateBuildDescription:retainBuildDescription:)")
266270
public init(sessionHandle: String, responseChannel: UInt64, request: BuildRequestMessagePayload, onlyCreateBuildDescription: Bool) {
267271
self.sessionHandle = sessionHandle
268272
self.responseChannel = responseChannel
269273
self.request = request
270274
self.onlyCreateBuildDescription = onlyCreateBuildDescription
275+
self.retainBuildDescription = nil
271276
}
272277

273-
public init(fromLegacy deserializer: any Deserializer) throws {
274-
try deserializer.beginAggregate(4)
275-
self.sessionHandle = try deserializer.deserialize()
276-
self.responseChannel = try deserializer.deserialize()
277-
self.request = try deserializer.deserialize()
278-
self.onlyCreateBuildDescription = try deserializer.deserialize()
278+
public init(sessionHandle: String, responseChannel: UInt64, request: BuildRequestMessagePayload, onlyCreateBuildDescription: Bool, retainBuildDescription: Bool) {
279+
self.sessionHandle = sessionHandle
280+
self.responseChannel = responseChannel
281+
self.request = request
282+
self.onlyCreateBuildDescription = onlyCreateBuildDescription
283+
self.retainBuildDescription = retainBuildDescription
279284
}
280285
}
281286

Sources/SWBTaskExecution/BuildDescriptionManager.swift

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ package final class BuildDescriptionManager: Sendable {
9090
/// The in-memory cache of build descriptions.
9191
private let inMemoryCachedBuildDescriptions: HeavyCache<BuildDescriptionSignature, BuildDescription>
9292

93+
/// Build descriptions explicitly retained by clients.
94+
private let retainedBuildDescriptions: Registry<BuildDescriptionSignature, (BuildDescription, UInt)> = .init()
95+
9396
/// The last build plan request. Used to generate a diff of the current plan for debugging purposes.
9497
private let lastBuildPlanRequest: SWBMutex<BuildPlanRequest?> = .init(nil)
9598

@@ -254,35 +257,44 @@ package final class BuildDescriptionManager: Sendable {
254257
/// During normal operation (outside of tests), this should always be called on `queue`.
255258
package enum BuildDescriptionRequest {
256259
/// Retrieve or create a build description based on a build plan.
257-
case newOrCached(BuildPlanRequest, bypassActualTasks: Bool, useSynchronousBuildDescriptionSerialization: Bool)
260+
case newOrCached(BuildPlanRequest, bypassActualTasks: Bool, useSynchronousBuildDescriptionSerialization: Bool, retain: Bool)
258261
/// Retrieve an existing build description, build planning has been avoided. If the build description is not available then `getNewOrCachedBuildDescription` will fail.
259-
case cachedOnly(BuildDescriptionID, request: BuildRequest, buildRequestContext: BuildRequestContext, workspaceContext: WorkspaceContext)
262+
case cachedOnly(BuildDescriptionID, request: BuildRequest, buildRequestContext: BuildRequestContext, workspaceContext: WorkspaceContext, retain: Bool)
260263

261264
var buildRequest: BuildRequest {
262265
switch self {
263-
case .newOrCached(let planRequest, _, _): return planRequest.buildRequest
264-
case .cachedOnly(_, let request, _, _): return request
266+
case .newOrCached(let planRequest, _, _, _): return planRequest.buildRequest
267+
case .cachedOnly(_, let request, _, _, _): return request
265268
}
266269
}
267270

268271
var buildRequestContext: BuildRequestContext {
269272
switch self {
270-
case .newOrCached(let planRequest, _, _): return planRequest.buildRequestContext
271-
case .cachedOnly(_, _, let buildRequestContext, _): return buildRequestContext
273+
case .newOrCached(let planRequest, _, _, _): return planRequest.buildRequestContext
274+
case .cachedOnly(_, _, let buildRequestContext, _, _): return buildRequestContext
272275
}
273276
}
274277

275278
var planRequest: BuildPlanRequest? {
276279
switch self {
277-
case .newOrCached(let planRequest, _, _): return planRequest
280+
case .newOrCached(let planRequest, _, _, _): return planRequest
278281
case .cachedOnly: return nil
279282
}
280283
}
281284

282285
var workspaceContext: WorkspaceContext {
283286
switch self {
284-
case .newOrCached(let planRequest, _, _): return planRequest.workspaceContext
285-
case .cachedOnly(_, _, _, let workspaceContext): return workspaceContext
287+
case .newOrCached(let planRequest, _, _, _): return planRequest.workspaceContext
288+
case .cachedOnly(_, _, _, let workspaceContext, _): return workspaceContext
289+
}
290+
}
291+
292+
var retain: Bool {
293+
switch self {
294+
case .newOrCached(_, _, _, let retain):
295+
retain
296+
case .cachedOnly(_, _, _, _, let retain):
297+
retain
286298
}
287299
}
288300

@@ -305,8 +317,8 @@ package final class BuildDescriptionManager: Sendable {
305317

306318
func signature(cacheDir: Path) throws -> BuildDescriptionSignature {
307319
switch self {
308-
case .newOrCached(let planRequest, _, _): return try BuildDescriptionSignature.buildDescriptionSignature(planRequest, cacheDir: cacheDir)
309-
case .cachedOnly(let buildDescriptionID, _, _, _): return BuildDescriptionSignature.buildDescriptionSignature(buildDescriptionID)
320+
case .newOrCached(let planRequest, _, _, _): return try BuildDescriptionSignature.buildDescriptionSignature(planRequest, cacheDir: cacheDir)
321+
case .cachedOnly(let buildDescriptionID, _, _, _, _): return BuildDescriptionSignature.buildDescriptionSignature(buildDescriptionID)
310322
}
311323
}
312324
}
@@ -317,6 +329,8 @@ package final class BuildDescriptionManager: Sendable {
317329
description = lastDescription
318330
} else if let inMemoryDescription = inMemoryCachedBuildDescriptions[signature] {
319331
description = inMemoryDescription
332+
} else if let retainedDescription = retainedBuildDescriptions[signature] {
333+
description = retainedDescription.0
320334
} else {
321335
description = nil
322336
}
@@ -397,18 +411,35 @@ package final class BuildDescriptionManager: Sendable {
397411
}
398412
}
399413

414+
if request.retain {
415+
retainedBuildDescriptions.update(signature, update: { ($0.0, $0.1 + 1) }, default: { (buildDescription, 0) })
416+
}
417+
400418
return BuildDescriptionRetrievalInfo(buildDescription: buildDescription, source: source, inMemoryCacheSize: inMemoryCachedBuildDescriptions.count, onDiskCachePath: buildDescriptionPath)
401419
}
402420

403421
/// Returns a build description for a particular workspace and request.
404422
///
405423
/// - Returns: A build description, or nil if cancelled.
406-
package func getBuildDescription(_ request: BuildPlanRequest, bypassActualTasks: Bool = false, useSynchronousBuildDescriptionSerialization: Bool = false, clientDelegate: any TaskPlanningClientDelegate, constructionDelegate: any BuildDescriptionConstructionDelegate) async throws -> BuildDescription? {
407-
let descRequest = BuildDescriptionRequest.newOrCached(request, bypassActualTasks: bypassActualTasks, useSynchronousBuildDescriptionSerialization: useSynchronousBuildDescriptionSerialization)
424+
package func getBuildDescription(_ request: BuildPlanRequest, bypassActualTasks: Bool = false, useSynchronousBuildDescriptionSerialization: Bool = false, retained: Bool, clientDelegate: any TaskPlanningClientDelegate, constructionDelegate: any BuildDescriptionConstructionDelegate) async throws -> BuildDescription? {
425+
let descRequest = BuildDescriptionRequest.newOrCached(request, bypassActualTasks: bypassActualTasks, useSynchronousBuildDescriptionSerialization: useSynchronousBuildDescriptionSerialization, retain: retained)
408426
let retrievalInfo = try await getNewOrCachedBuildDescription(descRequest, clientDelegate: clientDelegate, constructionDelegate: constructionDelegate)
409427
return retrievalInfo?.buildDescription
410428
}
411429

430+
package func releaseBuildDescription(id: BuildDescriptionID) {
431+
self.retainedBuildDescriptions.update(BuildDescriptionSignature.buildDescriptionSignature(id), update: {
432+
let newCount = $0.1 - 1
433+
if newCount == 0 {
434+
return nil
435+
} else {
436+
return ($0.0, newCount)
437+
}
438+
}, default: {
439+
nil
440+
})
441+
}
442+
412443
/// Returns the path in which the`XCBuildData` directory will live. That location is uses to cache build descriptions for a particular workspace and request, the manifest, and the `build.db` database for llbuild.
413444
package static func cacheDirectory(_ request: BuildPlanRequest) throws -> Path {
414445
return try cacheDirectory(request.buildRequest, buildRequestContext: request.buildRequestContext, workspaceContext: request.workspaceContext)
@@ -514,7 +545,7 @@ package final class BuildDescriptionManager: Sendable {
514545
}
515546

516547
// Unable to load from disk, create a new description
517-
guard case let .newOrCached(request, bypassActualTasks, useSynchronousBuildDescriptionSerialization) = request else {
548+
guard case let .newOrCached(request, bypassActualTasks, useSynchronousBuildDescriptionSerialization, _) = request else {
518549
preconditionFailure("entered build construction path but request was for existing cached description")
519550
}
520551

Sources/SWBTestSupport/TaskExecutionTestSupport.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ extension BuildDescription {
101101

102102
extension BuildDescriptionManager {
103103
package func getNewOrCachedBuildDescription(_ request: BuildPlanRequest, bypassActualTasks: Bool = false, clientDelegate: any TaskPlanningClientDelegate, constructionDelegate: any BuildDescriptionConstructionDelegate) async throws -> BuildDescriptionRetrievalInfo? {
104-
let descRequest = BuildDescriptionRequest.newOrCached(request, bypassActualTasks: bypassActualTasks, useSynchronousBuildDescriptionSerialization: true)
104+
let descRequest = BuildDescriptionRequest.newOrCached(request, bypassActualTasks: bypassActualTasks, useSynchronousBuildDescriptionSerialization: true, retain: false)
105105
return try await getNewOrCachedBuildDescription(descRequest, clientDelegate: clientDelegate, constructionDelegate: constructionDelegate)
106106
}
107107
}

0 commit comments

Comments
 (0)