Skip to content

Fix a couple of bugs with progress reporting in the source reducer #1159

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.

Sign up for GitHub

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 1 commit into from
Apr 5, 2024
Merged

Fix a couple of bugs with progress reporting in the source reducer #1159

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions Sources/Diagnose/SourceReducer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,10 @@ fileprivate class SourceReducer {

/// Replace the first `import` declaration in the source file by the contents of the Swift interface.
private func inlineFirstImport(_ requestInfo: RequestInfo) async throws -> RequestInfo? {
let reductionResult = try await runReductionStep(requestInfo: requestInfo) { tree in
// Don't report progress after inlining an import because it might increase the file size before we have a chance
// to increase `fileSizeAfterLastImportInline`. Progress will get updated again on the next successful reduction
// step.
let reductionResult = try await runReductionStep(requestInfo: requestInfo, reportProgress: false) { tree in
let edits = await Diagnose.inlineFirstImport(
in: tree,
executor: sourcekitdExecutor,
Expand All @@ -216,15 +219,22 @@ fileprivate class SourceReducer {
// MARK: Primitives to run reduction steps

private func logSuccessfulReduction(_ requestInfo: RequestInfo, tree: SourceFileSyntax) {
let numberOfImports = tree.numberOfImports
// The number of imports can grow if inlining a single module adds more than 1 new import.
// To keep progress between 0 and 1, clamp the number of imports to the initial import count.
let numberOfImports = min(tree.numberOfImports, initialImportCount)
let fileSize = requestInfo.fileContents.utf8.count

let progressPerRemovedImport = Double(1) / Double(initialImportCount + 1)
let removeImportProgress = Double(initialImportCount - numberOfImports) * progressPerRemovedImport
let fileReductionProgress =
(1 - Double(fileSize) / Double(fileSizeAfterLastImportInline)) * progressPerRemovedImport
let progress = removeImportProgress + fileReductionProgress
precondition(progress >= 0 && progress <= 1)
var progress = removeImportProgress + fileReductionProgress
if progress < 0 || progress > 1 {
logger.fault(
"Trying to report progress \(progress) from remove import progress \(removeImportProgress) and file reduction progress \(fileReductionProgress)"
)
progress = max(min(progress, 1), 0)
}
progressUpdate(progress, "Reduced to \(numberOfImports) imports and \(fileSize) bytes")
}

Expand All @@ -234,6 +244,7 @@ fileprivate class SourceReducer {
/// Otherwise, return `nil`
private func runReductionStep(
requestInfo: RequestInfo,
reportProgress: Bool = true,
reduce: (_ tree: SourceFileSyntax) async throws -> ReducerResult
) async throws -> ReductionStepResult {
let tree = Parser.parse(source: requestInfo.fileContents)
Expand Down Expand Up @@ -263,7 +274,9 @@ fileprivate class SourceReducer {
let result = try await sourcekitdExecutor.run(request: reducedRequestInfo)
if case .reproducesIssue = result {
logger.debug("Reduction successful")
logSuccessfulReduction(reducedRequestInfo, tree: tree)
if reportProgress {
logSuccessfulReduction(reducedRequestInfo, tree: tree)
}
return .reduced(reducedRequestInfo)
} else {
logger.debug("Reduction did not reproduce the issue")
Expand Down