Skip to content
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

Performance tweaks for larger solutions #210

Merged
merged 1 commit into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 12 additions & 1 deletion src/FSharp.Analyzers.Cli/CustomLogging.fs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,18 @@ type CustomFormatter(options: IOptionsMonitor<CustomOptions>) as this =
this.WritePrefix(textWriter, logEntry.LogLevel)
textWriter.WriteLine(message)

member private _.WritePrefix(textWriter: TextWriter, logLevel: LogLevel) =
member private x.WritePrefix(textWriter: TextWriter, logLevel: LogLevel) =
if not (isNull formatterOptions.TimestampFormat) then
let dateTime =
if formatterOptions.UseUtcTimestamp then
DateTime.UtcNow
else
DateTime.Now

let timestamp = dateTime.ToString(formatterOptions.TimestampFormat)

textWriter.Write($"{timestamp} ")
Comment on lines +43 to +52
Copy link
Member Author

Choose a reason for hiding this comment

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

Helped with diagnosing slow areas. Not exactly required for this but might be nice to keep.


match logLevel with
| LogLevel.Trace -> textWriter.Write("trace: ")
| LogLevel.Debug -> textWriter.Write("debug: ")
Expand Down
100 changes: 54 additions & 46 deletions src/FSharp.Analyzers.Cli/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ type Arguments =
interface IArgParserTemplate with
member s.Usage =
match s with
| Project _ -> "Path to your .fsproj file."
| Analyzers_Path _ -> "Path to a folder where your analyzers are located."
| Project _ -> "List of paths to your .fsproj file."
| Analyzers_Path _ ->
"List of path to a folder where your analyzers are located. This will search recursively."
Comment on lines +42 to +44
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed these took lists and should give help messages appropriate for them.

| Property _ -> "A key=value pair of an MSBuild property."
| Configuration _ -> "The configuration to use, e.g. Debug or Release."
| Runtime _ -> "The runtime identifier (RID)."
Expand Down Expand Up @@ -118,28 +119,39 @@ let rec mkKn (ty: Type) =

let mutable logger: ILogger = Abstractions.NullLogger.Instance

let loadProject toolsPath properties projPath =
let loadProjects toolsPath properties (projPaths: string list) =
async {
let projPaths =
projPaths
|> List.map (fun proj -> Path.Combine(Environment.CurrentDirectory, proj) |> Path.GetFullPath)

for proj in projPaths do
logger.LogInformation("Loading project {0}", proj)

let loader = WorkspaceLoader.Create(toolsPath, properties)
let parsed = loader.LoadProjects [ projPath ] |> Seq.toList
let projectOptions = loader.LoadProjects projPaths
Copy link
Member Author

@TheAngryByrd TheAngryByrd May 14, 2024

Choose a reason for hiding this comment

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

This is one of the major speed ups. Load all projects at once.

There's some locking that goes on behind the scenes when ProjInfo talks to msbuild becuase msbuild can only handle one build request at a time in process. It's best to front load as many project load requests as you can.


if parsed.IsEmpty then
logger.LogError("Failed to load project '{0}'", projPath)
exit 1
let failedLoads =
projPaths
|> Seq.filter (fun path -> not (projectOptions |> Seq.exists (fun p -> p.ProjectFileName = path)))
|> Seq.toList

let fcsPo = FCS.mapToFSharpProjectOptions parsed.Head parsed
if Seq.length failedLoads > 0 then
logger.LogError("Failed to load project '{0}'", failedLoads)
exit 1

return fcsPo
return FCS.mapManyOptions projectOptions |> Seq.toList
Copy link
Member Author

Choose a reason for hiding this comment

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

Another helper that maps to FCS types efficiently.

}

let runProjectAux
let runProject
Copy link
Member Author

@TheAngryByrd TheAngryByrd May 14, 2024

Choose a reason for hiding this comment

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

runProjectAux was no longer needed, renamed to runProject

(client: Client<CliAnalyzerAttribute, CliContext>)
(fsharpOptions: FSharpProjectOptions)
(excludeIncludeFiles: Choice<Glob list, Glob list>)
(mappings: SeverityMappings)
: Async<Result<AnalyzerMessage list, AnalysisFailure> list>
=
async {
logger.LogInformation("Checking project {0}", fsharpOptions.ProjectFileName)
let! checkProjectResults = fcs.ParseAndCheckProject(fsharpOptions)

let! messagesPerAnalyzer =
Expand All @@ -160,21 +172,23 @@ let runProjectAux
| None -> false
)
|> Array.map (fun fileName ->
let fileContent = File.ReadAllText fileName
let sourceText = SourceText.ofString fileContent
async {
let! fileContent = File.ReadAllTextAsync fileName |> Async.AwaitTask
let sourceText = SourceText.ofString fileContent
logger.LogDebug("Checking file {0}", fileName)

// Since we did ParseAndCheckProject, we can be sure that the file is in the project.
// See https://fsharp.github.io/fsharp-compiler-docs/fcs/project.html for more information.
let! parseAndCheckResults = fcs.GetBackgroundCheckResultsForFileInProject(fileName, fsharpOptions)
Copy link
Member Author

Choose a reason for hiding this comment

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

As the comment mentions, this helps with speedily getting the typecheck for that file since we already checked the project.


let ctx =
Utils.createContext checkProjectResults fileName sourceText parseAndCheckResults

logger.LogInformation("Running analyzers for {0}", ctx.FileName)
let! results = client.RunAnalyzers ctx
return Ok results
}

Utils.typeCheckFile fcs logger fsharpOptions fileName (Utils.SourceOfSource.SourceText sourceText)
|> Result.map (Utils.createContext checkProjectResults fileName sourceText)
)
|> Array.map (fun ctx ->
match ctx with
| Error e -> async.Return(Error e)
| Ok ctx ->
async {
logger.LogInformation("Running analyzers for {0}", ctx.FileName)
let! results = client.RunAnalyzers ctx
return Ok results
}
)
|> Async.Parallel

Expand All @@ -188,20 +202,6 @@ let runProjectAux
|> Seq.toList
}

let runProject
(client: Client<CliAnalyzerAttribute, CliContext>)
toolsPath
properties
proj
(excludeIncludeFiles: Choice<Glob list, Glob list>)
(mappings: SeverityMappings)
=
async {
let path = Path.Combine(Environment.CurrentDirectory, proj) |> Path.GetFullPath
let! option = loadProject toolsPath properties path
return! runProjectAux client option excludeIncludeFiles mappings
}

let fsharpFiles = set [| ".fs"; ".fsi"; ".fsx" |]

let isFSharpFile (file: string) =
Expand Down Expand Up @@ -249,7 +249,7 @@ let runFscArgs
Stamp = None
}

runProjectAux client projectOptions excludeIncludeFiles mappings
runProject client projectOptions excludeIncludeFiles mappings

let printMessages (msgs: AnalyzerMessage list) =

Expand Down Expand Up @@ -482,7 +482,11 @@ let main argv =
use factory =
LoggerFactory.Create(fun builder ->
builder
.AddCustomFormatter(fun options -> options.UseAnalyzersMsgStyle <- false)
.AddCustomFormatter(fun options ->
options.UseAnalyzersMsgStyle <- false
options.TimestampFormat <- "[HH:mm:ss.fff]"
options.UseUtcTimestamp <- true
)
.SetMinimumLevel(logLevel)
|> ignore
)
Expand Down Expand Up @@ -610,7 +614,6 @@ let main argv =
match projOpts, fscArgs with
| [], None ->
logger.LogError("No project given. Use `--project PATH_TO_FSPROJ`.")

None
| _ :: _, Some _ ->
logger.LogError("`--project` and `--fsc-args` cannot be combined.")
Expand All @@ -625,11 +628,16 @@ let main argv =
logger.LogError("Invalid `--project` argument. File does not exist: '{projPath}'", projPath)
exit 1

projects
|> List.map (fun projPath ->
runProject client toolsPath properties projPath exclInclFiles severityMapping
)
|> Async.Sequential
async {
let! loadedProjects = loadProjects toolsPath properties projects

return!
loadedProjects
|> List.map (fun (projPath: FSharpProjectOptions) ->
runProject client projPath exclInclFiles severityMapping
)
|> Async.Parallel
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we front load all projects we can use Async.Parallel to schedule as all the operations.

}
|> Async.RunSynchronously
|> List.concat
|> Some
Expand Down
Loading