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

Use new GetProjectDirectory method to reduce string allocations #9603

Merged
merged 2 commits into from
Dec 2, 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
4 changes: 2 additions & 2 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
<PackageVersion Include="Microsoft.VisualStudio.ImageCatalog" Version="17.13.38055-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.Interop" Version="17.13.38055-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.ManagedInterfaces" Version="17.13.38047-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.12.12" />
<PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.13.3-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Settings.15.0" Version="17.13.38055-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.Setup.Configuration.Interop" Version="3.12.2159" />
<PackageVersion Include="Microsoft.VisualStudio.Shell.15.0" Version="17.13.38055-preview.1" />
Expand All @@ -65,7 +65,7 @@
<PackageVersion Include="Microsoft.VisualStudio.TemplateWizardInterface" Version="17.13.38055-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.Threading" Version="17.12.19" />
<PackageVersion Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.12.19" />
<PackageVersion Include="Microsoft.VisualStudio.Utilities" Version="17.13.38055-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.Utilities" Version="17.13.38299-preview.1" />
<PackageVersion Include="Microsoft.VisualStudio.Validation" Version="17.8.8" />
<PackageVersion Include="Microsoft.VisualStudio.XmlEditor" Version="17.13.0-preview-1-35408-014" />
<PackageVersion Include="Microsoft.VSSDK.BuildTools" Version="17.13.17-preview1-ga15b669c04" />
Expand Down
2 changes: 1 addition & 1 deletion eng/imports/Versions.props
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!-- Any changes to this file or format requires updates in project-system-vscode -->
<Project>
<PropertyGroup>
<CPSPackageVersion>17.13.16-pre</CPSPackageVersion>
<CPSPackageVersion>17.13.42-pre</CPSPackageVersion>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private async Task<bool> IsIntegratedConsoleEnabledAsync()

// If no working directory specified in the profile, we default to output directory. If for some reason the output directory
// is not specified, fall back to the project folder.
string projectFolderFullPath = Path.GetDirectoryName(_project.UnconfiguredProject.FullPath) ?? string.Empty;
string projectFolderFullPath = _project.UnconfiguredProject.GetProjectDirectory();
string defaultWorkingDir = await ProjectAndExecutableLaunchHandlerHelpers.GetDefaultWorkingDirectoryAsync(configuredProject, projectFolderFullPath, _fileSystem);

string? commandLineArgs = resolvedProfile.CommandLineArgs is null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void AddToContextIfNotPresent(string includePath, IImmutableDictionary<string, s

if (!_paths.Contains(fullPath))
{
string[]? folderNames = FileItemServices.GetLogicalFolderNames(Path.GetDirectoryName(project.FullPath), fullPath, metadata);
string[]? folderNames = FileItemServices.GetLogicalFolderNames(project.GetProjectDirectory(), fullPath, metadata);

logger.WriteLine("Adding dynamic file '{0}'", fullPath);
context.AddDynamicFile(fullPath, folderNames);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ internal enum WorkspaceState
private readonly JoinableTaskCollection _joinableTaskCollection;
private readonly JoinableTaskFactory _joinableTaskFactory;
private readonly CancellationToken _unloadCancellationToken;
private readonly string _baseDirectory;

/// <summary>Completes when the workspace has integrated build data.</summary>
private readonly TaskCompletionSource _contextCreated = new(TaskCreationOptions.RunContinuationsAsynchronously);
Expand Down Expand Up @@ -118,8 +117,6 @@ internal Workspace(
_joinableTaskFactory = joinableTaskFactory;
_unloadCancellationToken = unloadCancellationToken;

_baseDirectory = Path.GetDirectoryName(_unconfiguredProject.FullPath);

// We take ownership of the lifetime of the provided update handlers, and dispose them
// when this workspace is disposed.
_disposableBag = new() { updateHandlers };
Expand Down Expand Up @@ -453,8 +450,10 @@ void InvokeCommandLineUpdateHandlers()

IComparable version = GetConfiguredProjectVersion(update);

BuildOptions added = parser.Parse(projectChange.Difference.AddedItems, _baseDirectory);
BuildOptions removed = parser.Parse(projectChange.Difference.RemovedItems, _baseDirectory);
string baseDirectory = _unconfiguredProject.GetProjectDirectory();

BuildOptions added = parser.Parse(projectChange.Difference.AddedItems, baseDirectory);
BuildOptions removed = parser.Parse(projectChange.Difference.RemovedItems, baseDirectory);

foreach (ICommandLineHandler commandLineHandler in _updateHandlers.CommandLineHandlers)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ private void WatchLaunchSettingsFile()

try
{
FileWatcher = new SimpleFileWatcher(Path.GetDirectoryName(_commonProjectServices.Project.FullPath),
FileWatcher = new SimpleFileWatcher(_commonProjectServices.Project.GetProjectDirectory(),
true,
NotifyFilters.FileName | NotifyFilters.Size | NotifyFilters.LastWrite,
LaunchSettingsFilename,
Expand Down Expand Up @@ -894,7 +894,7 @@ internal async Task<string> GetLaunchSettingsFilePathNoCacheAsync()
// see: https://github.com/dotnet/project-system/issues/2316.

// Default to the project directory if we're not able to get the AppDesigner folder.
string folder = Path.GetDirectoryName(_commonProjectServices.Project.FullPath);
string folder = _commonProjectServices.Project.GetProjectDirectory();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the new GetProjectDirectory() from CPS works cross-platform?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's in the host-agnostic layer so is available outside of VS too.

The CPS PRs were:


if (_projectProperties.Value is not null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public static async Task<string> GetDefaultWorkingDirectoryAsync(ConfiguredProje
// If the working directory is relative, it will be relative to the project root so make it a full path
if (!string.IsNullOrWhiteSpace(runWorkingDirectory) && !Path.IsPathRooted(runWorkingDirectory))
{
runWorkingDirectory = Path.Combine(Path.GetDirectoryName(project.UnconfiguredProject.FullPath) ?? string.Empty, runWorkingDirectory);
runWorkingDirectory = Path.Combine(project.UnconfiguredProject.GetProjectDirectory(), runWorkingDirectory);
}

string runArguments = await properties.GetEvaluatedPropertyValueAsync("RunArguments");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public ApplicationIconValueProvider(
try
{
propertyValue = Path.GetFileName(unevaluatedPropertyValue);
var destinationInfo = new FileInfo(Path.Combine(Path.GetDirectoryName(_unconfiguredProject.FullPath), propertyValue));
var destinationInfo = new FileInfo(Path.Combine(_unconfiguredProject.GetProjectDirectory(), propertyValue));
if (destinationInfo.Exists && destinationInfo.IsReadOnly)
{
// The file cannot be copied over; return the previous value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public async Task QueryDebugTargetsAsync_ExeProfileAsyncExeRelativeNoWorkingDir(
if (outdir is null || outdir == @"doesntExist\")
{
Assert.Equal(@"c:\test\project\test.exe", targets[0].Executable);
Assert.Equal(@"c:\test\project", targets[0].CurrentDirectory);
Assert.Equal(@"c:\test\project\", targets[0].CurrentDirectory);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ public void UniqueSourceFilesPushedToWorkspace()
void onSourceFileAdded(string s) => Assert.True(sourceFilesPushedToWorkspace.Add(s));
void onSourceFileRemoved(string s) => sourceFilesPushedToWorkspace.Remove(s);

var project = UnconfiguredProjectFactory.Create(fullPath: @"C:\Myproject.csproj");
var project = UnconfiguredProjectFactory.Create(fullPath: @"C:\MyProject.csproj");
var context = IWorkspaceProjectContextMockFactory.CreateForSourceFiles(project, onSourceFileAdded, onSourceFileRemoved);
var logger = Mock.Of<IManagedProjectDiagnosticOutputService>();

var handler = new CompileItemHandler(project);
var projectDir = Path.GetDirectoryName(project.FullPath);
var added = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new[] { @"C:\file1.cs", @"C:\file2.cs", @"C:\file1.cs" }, baseDirectory: projectDir, sdkDirectory: null));
var empty = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new string[] { }, baseDirectory: projectDir, sdkDirectory: null));
var projectDir = project.GetProjectDirectory();
var added = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: [@"C:\file1.cs", @"C:\file2.cs", @"C:\file1.cs"], baseDirectory: projectDir, sdkDirectory: null));
var empty = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: [], baseDirectory: projectDir, sdkDirectory: null));

handler.Handle(context, 10, added: added, removed: empty, new ContextState(isActiveEditorContext: true, isActiveConfiguration: false), logger: logger);

AssertEx.CollectionLength(sourceFilesPushedToWorkspace, 2);
Assert.Contains(@"C:\file1.cs", sourceFilesPushedToWorkspace);
Assert.Contains(@"C:\file2.cs", sourceFilesPushedToWorkspace);

var removed = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new[] { @"C:\file1.cs", @"C:\file1.cs" }, baseDirectory: projectDir, sdkDirectory: null));
var removed = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: [@"C:\file1.cs", @"C:\file1.cs"], baseDirectory: projectDir, sdkDirectory: null));
handler.Handle(context, 10, added: empty, removed: removed, new ContextState(isActiveEditorContext: true, isActiveConfiguration: false), logger: logger);

Assert.Single(sourceFilesPushedToWorkspace);
Expand All @@ -52,14 +52,14 @@ public void RootedSourceFilesPushedToWorkspace()
void onSourceFileAdded(string s) => Assert.True(sourceFilesPushedToWorkspace.Add(s));
void onSourceFileRemoved(string s) => sourceFilesPushedToWorkspace.Remove(s);

var project = UnconfiguredProjectFactory.Create(fullPath: @"C:\ProjectFolder\Myproject.csproj");
var project = UnconfiguredProjectFactory.Create(fullPath: @"C:\ProjectFolder\MyProject.csproj");
var context = IWorkspaceProjectContextMockFactory.CreateForSourceFiles(project, onSourceFileAdded, onSourceFileRemoved);
var logger = Mock.Of<IManagedProjectDiagnosticOutputService>();

var handler = new CompileItemHandler(project);
var projectDir = Path.GetDirectoryName(project.FullPath);
var added = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new[] { @"file1.cs", @"..\ProjectFolder\file1.cs" }, baseDirectory: projectDir, sdkDirectory: null));
var removed = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new string[] { }, baseDirectory: projectDir, sdkDirectory: null));
var projectDir = project.GetProjectDirectory();
var added = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: [@"file1.cs", @"..\ProjectFolder\file1.cs"], baseDirectory: projectDir, sdkDirectory: null));
var removed = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: [], baseDirectory: projectDir, sdkDirectory: null));

handler.Handle(context, 10, added: added, removed: removed, new ContextState(true, false), logger: logger);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void DuplicateMetadataReferencesPushedToWorkspace()
var logger = Mock.Of<IManagedProjectDiagnosticOutputService>();

var handler = new MetadataReferenceItemHandler(project);
var projectDir = Path.GetDirectoryName(project.FullPath);
var projectDir = project.GetProjectDirectory();
var added = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new[] { @"/reference:C:\Assembly1.dll", @"/reference:C:\Assembly2.dll", @"/reference:C:\Assembly1.dll" }, baseDirectory: projectDir, sdkDirectory: null));
var empty = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new string[] { }, baseDirectory: projectDir, sdkDirectory: null));

Expand Down Expand Up @@ -48,7 +48,7 @@ public void RootedReferencesPushedToWorkspace()
var logger = Mock.Of<IManagedProjectDiagnosticOutputService>();

var handler = new MetadataReferenceItemHandler(project);
var projectDir = Path.GetDirectoryName(project.FullPath);
var projectDir = project.GetProjectDirectory();
var added = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new[] { @"/reference:Assembly1.dll", @"/reference:C:\ProjectFolder\Assembly2.dll", @"/reference:..\ProjectFolder\Assembly3.dll" }, baseDirectory: projectDir, sdkDirectory: null));
var removed = BuildOptions.FromCommandLineArguments(CSharpCommandLineParser.Default.Parse(args: new string[] { }, baseDirectory: projectDir, sdkDirectory: null));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ public async Task EvaluationUpdate_InitializesWorkspace()
workspaceProjectContextFactory.VerifyAll();
workspaceProjectContext.VerifyAll();
unconfiguredProjectServices.VerifyAll();
unconfiguredProject.VerifyAll();
unconfiguredProject.Verify();

Assert.Same(workspaceProjectContext.Object, workspace.Context);
}
Expand Down Expand Up @@ -754,7 +754,7 @@ private async Task EvaluationUpdateThrowsAndDisposesAsync(Action<ISetup<IWorkspa

workspaceProjectContextFactory.VerifyAll();
unconfiguredProjectServices.VerifyAll();
unconfiguredProject.VerifyAll();
unconfiguredProject.Verify();
}

[Theory] // Configurations Project GUID Expected
Expand Down Expand Up @@ -808,12 +808,12 @@ private static async Task<Workspace> CreateInstanceAsync(
IProjectSubscriptionUpdate? buildRuleUpdate = null)
{
var commandLineParserService = new Mock<ICommandLineParserService>(MockBehavior.Strict);
commandLineParserService.Setup(o => o.Parse(It.IsAny<IEnumerable<string>>(), """C:\MyProject""")).Returns(EmptyBuildOptions);
commandLineParserService.Setup(o => o.Parse(It.IsAny<IEnumerable<string>>(), """C:\MyProject\""")).Returns(EmptyBuildOptions);

slice ??= ProjectConfigurationSlice.Create(ImmutableStringDictionary<string>.EmptyOrdinal.Add("TargetFramework", "net6.0"));
unconfiguredProject ??= UnconfiguredProjectFactory.ImplementFullPath("""C:\MyProject\MyProject.csproj""");
projectGuid ??= Guid.NewGuid();
updateHandlers ??= new UpdateHandlers(Array.Empty<ExportFactory<IWorkspaceUpdateHandler>>());
updateHandlers ??= new UpdateHandlers([]);
logger ??= IManagedProjectDiagnosticOutputServiceFactory.Create();
activeWorkspaceProjectContextTracker ??= IActiveEditorContextTrackerFactory.Create();
commandLineParserServices ??= new(ImportOrderPrecedenceComparer.PreferenceOrder.PreferredComesFirst) { commandLineParserService.Object };
Expand Down