Skip to content

Commit e7069f2

Browse files
authored
Update targets to reduce syntax tree recreations due to ParseOption changes (#78093)
Additionally, reduce false positives in telemetry reporting inefficiencies in project system interactions wrt Preprocessor directives.
2 parents 48abcb5 + 603d832 commit e7069f2

File tree

4 files changed

+28
-5
lines changed

4 files changed

+28
-5
lines changed

src/Compilers/Core/MSBuildTask/Microsoft.CSharp.Core.targets

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,5 +199,7 @@
199199
<CommandLineArgsForDesignTimeEvaluation>-langversion:$(LangVersion)</CommandLineArgsForDesignTimeEvaluation>
200200
<CommandLineArgsForDesignTimeEvaluation Condition="'$(ChecksumAlgorithm)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -checksumalgorithm:$(ChecksumAlgorithm)</CommandLineArgsForDesignTimeEvaluation>
201201
<CommandLineArgsForDesignTimeEvaluation Condition="'$(DefineConstants)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -define:$(DefineConstants)</CommandLineArgsForDesignTimeEvaluation>
202+
<CommandLineArgsForDesignTimeEvaluation Condition="'$(Features)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -features:$(Features)</CommandLineArgsForDesignTimeEvaluation>
203+
<CommandLineArgsForDesignTimeEvaluation Condition="'$(DocumentationFile)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -doc:"$(DocumentationFile)"</CommandLineArgsForDesignTimeEvaluation>
202204
</PropertyGroup>
203205
</Project>

src/Compilers/Core/MSBuildTask/Microsoft.VisualBasic.Core.targets

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,5 +164,7 @@
164164
<CommandLineArgsForDesignTimeEvaluation>-langversion:$(LangVersion)</CommandLineArgsForDesignTimeEvaluation>
165165
<CommandLineArgsForDesignTimeEvaluation Condition="'$(ChecksumAlgorithm)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -checksumalgorithm:$(ChecksumAlgorithm)</CommandLineArgsForDesignTimeEvaluation>
166166
<CommandLineArgsForDesignTimeEvaluation Condition="'$(FinalDefineConstants)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -define:$(FinalDefineConstants)</CommandLineArgsForDesignTimeEvaluation>
167+
<CommandLineArgsForDesignTimeEvaluation Condition="'$(Features)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -features:$(Features)</CommandLineArgsForDesignTimeEvaluation>
168+
<CommandLineArgsForDesignTimeEvaluation Condition="'$(DocumentationFile)' != ''">$(CommandLineArgsForDesignTimeEvaluation) -doc:"$(DocumentationFile)"</CommandLineArgsForDesignTimeEvaluation>
167169
</PropertyGroup>
168170
</Project>

src/VisualStudio/Core/Impl/ProjectSystem/CPS/CPSProjectFactory.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public async Task<IWorkspaceProjectContext> CreateProjectContextAsync(Guid id, s
8484

8585
// Property added in VS 17.4 compiler targets capturing values of LangVersion and DefineConstants.
8686
// ChecksumAlgorithm value added to the property in 17.5.
87+
// Features and DocumentationFile added after 17.14.
8788
//
8889
// Impact on Hot Reload: incorrect ChecksumAlgorithm will prevent Hot Reload in detecting changes correctly in certain scenarios.
8990
// However, given that projects that explicitly set ChecksumAlgorithm to a non-default value are rare and the project system

src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ namespace Microsoft.CodeAnalysis.Workspaces.ProjectSystem;
3131

3232
internal sealed partial class ProjectSystemProject
3333
{
34-
private static readonly char[] s_directorySeparator = [Path.DirectorySeparatorChar];
3534
private static readonly ImmutableArray<MetadataReferenceProperties> s_defaultMetadataReferenceProperties = [default(MetadataReferenceProperties)];
3635

3736
private readonly ProjectSystemProjectFactory _projectSystemProjectFactory;
@@ -264,6 +263,9 @@ private void ChangeProjectProperty<T>(
264263

265264
field = newValue;
266265

266+
_projectPropertyModificationsInBatch.Add(
267+
(solutionChanges, projectUpdateState) => updateSolution(solutionChanges, projectUpdateState, oldValue));
268+
267269
if (logThrowAwayTelemetry)
268270
{
269271
var telemetryService = _projectSystemProjectFactory.SolutionServices.GetService<IWorkspaceTelemetryService>();
@@ -284,13 +286,29 @@ private void ChangeProjectProperty<T>(
284286

285287
if (!isFullyLoaded)
286288
{
287-
TryReportCompilationThrownAway(_projectSystemProjectFactory.Workspace.CurrentSolution, Id);
289+
if (ShouldReportCompilationWillBeThrownAway(newValue, oldValue))
290+
TryReportCompilationThrownAway(_projectSystemProjectFactory.Workspace.CurrentSolution, Id);
288291
}
289292
}
290293
}
294+
}
291295

292-
_projectPropertyModificationsInBatch.Add(
293-
(solutionChanges, projectUpdateState) => updateSolution(solutionChanges, projectUpdateState, oldValue));
296+
bool ShouldReportCompilationWillBeThrownAway(T newValue, T? oldValue)
297+
{
298+
// This method can be called for both AssemblyName (type string) and ParseOptions (type ParseOptions) changes.
299+
300+
// We report telemetry for AssemblyName changes.
301+
if (newValue is not ParseOptions newParseOption)
302+
return true;
303+
304+
// We don't want to report telemetry for the initial evaluation result.
305+
if (oldValue is not ParseOptions oldParseOptions)
306+
return false;
307+
308+
// ParseOptions should be reported if they differ by more than just preprocessor directives. (See DocumentState.UpdateParseOptionsAndSourceCodeKind)
309+
var syntaxTreeFactoryService = _projectSystemProjectFactory.SolutionServices.GetRequiredLanguageService<ISyntaxTreeFactoryService>(Language);
310+
311+
return !syntaxTreeFactoryService.OptionsDifferOnlyByPreprocessorDirectives(oldParseOptions, newParseOption);
294312
}
295313
}
296314

@@ -361,7 +379,7 @@ public string AssemblyName
361379
public CompilationOptions? CompilationOptions
362380
{
363381
get => _compilationOptions;
364-
set => ChangeProjectProperty(ref _compilationOptions, value, s => s.WithProjectCompilationOptions(Id, value), logThrowAwayTelemetry: true);
382+
set => ChangeProjectProperty(ref _compilationOptions, value, s => s.WithProjectCompilationOptions(Id, value));
365383
}
366384

367385
// The property could be null if this is a non-C#/VB language and we don't have one for it. But we disallow assigning null, because C#/VB cannot end up null

0 commit comments

Comments
 (0)