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.

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

Fix slow graph binlog #226

Merged
merged 2 commits into from
Feb 18, 2025
Merged

Fix slow graph binlog #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8723c7
chore: add spelling
TheAngryByrd Feb 18, 2025
49c5aa3
fix: slow binlog writes for graph based builds
TheAngryByrd Feb 18, 2025
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
2 changes: 2 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
"cSpell.words": [
"binlog",
"inheritdoc",
"Ionide",
"msbuild",
"tfms",
"vswhere",
"xbuild"
Expand Down
26 changes: 8 additions & 18 deletions src/Ionide.ProjInfo/Library.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ module ProjectLoader =
let pi = project.CreateProjectInstance()
getTfm pi isLegacyFrameworkProj

let createLoggers (paths: string seq) (binaryLogs: BinaryLogGeneration) (sw: StringWriter) =
let createLoggers (path: string) (binaryLogs: BinaryLogGeneration) (sw: StringWriter) =
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 both call sites were only passing in a single path I changed this. Though it is public so would be a breaking change if someone was using it.

let swLogger = stringWriterLogger (sw)
let msBuildLogger = msBuildToLogProvider ()

Expand All @@ -541,24 +541,14 @@ module ProjectLoader =
let logFileName = Path.ChangeExtension(projectFileName, ".binlog")
Path.Combine(dir.FullName, logFileName)

match binaryLogs with
| BinaryLogGeneration.Off -> [
[
swLogger
msBuildLogger
]
| BinaryLogGeneration.Within dir ->
let loggers =
paths
|> Seq.map (fun path ->
let logPath = logFilePath (dir, path)
Microsoft.Build.Logging.BinaryLogger(Parameters = logPath) :> ILogger
)
match binaryLogs with
| BinaryLogGeneration.Off -> ()
| BinaryLogGeneration.Within dir -> Microsoft.Build.Logging.BinaryLogger(Parameters = logFilePath (dir, path)) :> ILogger

[
swLogger
msBuildLogger
yield! loggers
]
]

let getGlobalProps (tfm: string option) (globalProperties: (string * string) list) (propsSetFromParentCollection: Set<string>) =
[
Expand Down Expand Up @@ -655,7 +645,7 @@ module ProjectLoader =
let project = findOrCreateMatchingProject path projectCollection globalProperties
use sw = new StringWriter()

let loggers = createLoggers [ path ] binaryLogs sw
let loggers = createLoggers path binaryLogs sw

let pi = project.CreateProjectInstance()
let designTimeTargets = designTimeBuildTargets isLegacyFrameworkProjFile
Expand Down Expand Up @@ -1224,7 +1214,7 @@ type WorkspaceLoaderViaProjectGraph private (toolsPath, ?globalProperties: (stri

let bm = BuildManager.DefaultBuildManager
use sw = new StringWriter()
let loggers = ProjectLoader.createLoggers allKnownNames binaryLogs sw
let loggers = ProjectLoader.createLoggers "graph-build" binaryLogs sw
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously we'd create a binlog for each project that we were design time building. This would cause excessive slowdowns.

let buildParameters = BuildParameters(Loggers = loggers)

buildParameters.ProjectLoadSettings <-
Expand Down
8 changes: 4 additions & 4 deletions test/Ionide.ProjInfo.Tests/Tests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1577,7 +1577,7 @@ let testFCSmapManyProjCheckCaching =
Expect.equal distinctOptionsCount projectsInLayers.Length "Mapping should reuse instances of FSharpProjectOptions and only create one per project"
)

let testSample2WithBinLog toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorkspaceLoader) =
let testSample2WithBinLog binLogFile toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorkspaceLoader) =
testCase
|> withLog
(sprintf "can load sample2 with bin log - %s" workspaceLoader)
Expand Down Expand Up @@ -1630,7 +1630,7 @@ let testSample2WithBinLog toolsPath workspaceLoader (workspaceFactory: ToolsPath

let blPath =
projDir
/ "n1.binlog"
/ binLogFile

let blExists = File.Exists blPath

Expand Down Expand Up @@ -2364,8 +2364,8 @@ let tests toolsPath =
loadProjfileFromDiskTests toolsPath "WorkspaceLoaderViaProjectGraph" WorkspaceLoaderViaProjectGraph.Create

//Binlog test
testSample2WithBinLog toolsPath "WorkspaceLoader" WorkspaceLoader.Create
testSample2WithBinLog toolsPath "WorkspaceLoaderViaProjectGraph" WorkspaceLoaderViaProjectGraph.Create
testSample2WithBinLog "n1.binlog" toolsPath "WorkspaceLoader" WorkspaceLoader.Create
testSample2WithBinLog "graph-build.binlog" toolsPath "WorkspaceLoaderViaProjectGraph" WorkspaceLoaderViaProjectGraph.Create
test "can get runtimes" {
let runtimes =
SdkDiscovery.runtimes (
Expand Down