mfogliatto / mfogliatto/ReferenceCop
[Performance] MSBuildProjectMetadataProvider creates redundant ProjectCollection instances and leaks IDisposable
- Dominant language
- C#
- Stars
- 1
- Forks
- 2
- PR merge metrics
- No merged PRs in 30d
Description
## Description
`MSBuildProjectMetadataProvider` creates a new `ProjectCollection()` and calls `LoadProject()` in **both** `GetProjectReferences()` and `GetPropertyValue()`. In `ReferenceCopTask.Execute()`, both methods are called for the same project file, meaning the project is loaded and parsed twice per task invocation.
Additionally, `ProjectCollection` implements `IDisposable` but is never disposed, leaking unmanaged resources.
## Affected Files
- `src/ReferenceCop.MSBuild/Providers/MSBuildProjectMetadataProvider.cs` — lines 22-23 and 50-51
- `src/ReferenceCop.MSBuild/ReferenceCopTask.cs` — `Execute()` calls both methods sequentially
## Impact
- **Double project load**: `ProjectCollection.LoadProject()` involves full MSBuild project evaluation (parsing XML, evaluating properties, resolving imports). Doing this twice for the same file is wasteful.
- **Resource leak**: `ProjectCollection` is `IDisposable` and holds unmanaged resources (COM interop, file handles). Never disposing it causes resource leaks, especially across a multi-project build where the MSBuild task runs per-project.
- **Build-time cost**: This compounds across a solution — for N projects, there are 2N unnecessary project evaluations.
## Suggested Optimization
Share a single `ProjectCollection` instance per method call, or refactor to load the project once and extract both references and properties:
```csharp
public class MSBuildProjectMetadataProvider : IProjectMetadataProvider, IDisposable
{
private readonly ProjectCollection projectCollection = new ProjectCollection();
public IEnumerable GetProjectReferences(string projectFilePath)
{
var project = this.projectCollection.LoadProject(projectFilePath);
// ... extract references ...
}
public string GetPropertyValue(string projectFilePath, string propertyName)
{
// Reuse already-loaded project from the collection
var project = this.projectCollection.GetLoadedProjects(projectFilePath).FirstOrDefault()
?? this.projectCollection.LoadProject(projectFilePath);
project.ReevaluateIfNecessary();
return project.GetPropertyValue(propertyName);
}
public void Dispose()
{
this.projectCollection.Dispose();
}
}
```
Alternatively, a simpler approach: load the project once in `ReferenceCopTask.Execute()` and pass the loaded `Project` object to both operations.
Contributor guide
Research direction
Start with src/ReferenceCop.MSBuild/Providers/MSBuildProjectMetadataProvider.cs and inspect how GetProjectReferences() and GetPropertyValue() create and load ProjectCollection instances. Then read ReferenceCopTask.Execute() to trace their sequential calls for one project. Done means the project is not evaluated redundantly and each ProjectCollection lifecycle is handled without leaking resources.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100