mfogliatto / mfogliatto/ReferenceCop

[Performance] MSBuildProjectMetadataProvider creates redundant ProjectCollection instances and leaks IDisposable

Open
#68 0 comments 0 reactions 0 assignees View on GitHub
performance
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.