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

Cache internal objects used by the formatting engine. #73487

Merged
merged 5 commits into from
May 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public CSharpFormatEngine(
internal override IHeaderFacts HeaderFacts => CSharpHeaderFacts.Instance;

protected override AbstractTriviaDataFactory CreateTriviaFactory()
=> new TriviaDataFactory(this.TreeData, this.Options);
=> new TriviaDataFactory(this.TreeData, this.Options.LineFormatting);

protected override AbstractFormattingResult CreateFormattingResult(TokenStream tokenStream)
=> new FormattingResult(this.TreeData, tokenStream, this.SpanToFormat);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private CSharpStructuredTriviaFormatEngine(
internal override IHeaderFacts HeaderFacts => CSharpHeaderFacts.Instance;

protected override AbstractTriviaDataFactory CreateTriviaFactory()
=> new TriviaDataFactory(this.TreeData, this.Options);
=> new TriviaDataFactory(this.TreeData, this.Options.LineFormatting);

protected override FormattingContext CreateFormattingContext(TokenStream tokenStream, CancellationToken cancellationToken)
=> new(this, tokenStream);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ internal partial class TriviaDataFactory
/// represents a general trivia between two tokens. slightly more expensive than others since it
/// needs to calculate stuff unlike other cases
/// </summary>
private class ComplexTrivia : AbstractComplexTrivia
private sealed class ComplexTrivia : AbstractComplexTrivia
{
public ComplexTrivia(SyntaxFormattingOptions options, TreeData treeInfo, SyntaxToken token1, SyntaxToken token2)
public ComplexTrivia(LineFormattingOptions options, TreeData treeInfo, SyntaxToken token1, SyntaxToken token2)
: base(options, treeInfo, token1, token2)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Formatting;

internal partial class TriviaDataFactory
{
private class FormattedComplexTrivia : TriviaDataWithList
private sealed class FormattedComplexTrivia : TriviaDataWithList
{
private readonly CSharpTriviaFormatter _formatter;
private readonly IList<TextChange> _textChanges;
Expand All @@ -27,7 +27,7 @@ public FormattedComplexTrivia(
int spaces,
string originalString,
CancellationToken cancellationToken)
: base(context.Options, LanguageNames.CSharp)
: base(context.Options.LineFormatting)
{
Contract.ThrowIfNull(context);
Contract.ThrowIfNull(formattingRules);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ namespace Microsoft.CodeAnalysis.CSharp.Formatting;

internal partial class TriviaDataFactory
{
private class ModifiedComplexTrivia : TriviaDataWithList
private sealed class ModifiedComplexTrivia : TriviaDataWithList
{
private readonly ComplexTrivia _original;

public ModifiedComplexTrivia(SyntaxFormattingOptions options, ComplexTrivia original, int lineBreaks, int space)
: base(options, original.Token1.Language)
public ModifiedComplexTrivia(LineFormattingOptions options, ComplexTrivia original, int lineBreaks, int space)
: base(options)
{
Contract.ThrowIfNull(original);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Formatting;
/// </summary>
internal partial class TriviaDataFactory : AbstractTriviaDataFactory
{
public TriviaDataFactory(TreeData treeInfo, SyntaxFormattingOptions options)
public TriviaDataFactory(TreeData treeInfo, LineFormattingOptions options)
: base(treeInfo, options)
{
}
Expand Down Expand Up @@ -118,7 +118,7 @@ private static bool ContainsOnlyWhitespace(Analyzer.AnalysisResult result)
{
// calculate actual space size from tab
var spaces = CalculateSpaces(token1, token2);
return new ModifiedWhitespace(this.Options, result.LineBreaks, indentation: spaces, elastic: result.TreatAsElastic, language: LanguageNames.CSharp);
return new ModifiedWhitespace(this.Options, result.LineBreaks, indentation: spaces, elastic: result.TreatAsElastic);
}

// check whether we can cache trivia info for current indentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.

using System.Threading;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Roslyn.Utilities;

Expand All @@ -21,8 +20,8 @@ protected abstract class AbstractComplexTrivia : TriviaDataWithList

private readonly bool _treatAsElastic;

public AbstractComplexTrivia(SyntaxFormattingOptions options, TreeData treeInfo, SyntaxToken token1, SyntaxToken token2)
: base(options, token1.Language)
public AbstractComplexTrivia(LineFormattingOptions options, TreeData treeInfo, SyntaxToken token1, SyntaxToken token2)
: base(options)
{
Contract.ThrowIfNull(treeInfo);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ namespace Microsoft.CodeAnalysis.Formatting;

internal abstract partial class AbstractTriviaDataFactory
{
protected class FormattedWhitespace : TriviaData
protected sealed class FormattedWhitespace : TriviaData
{
private readonly string _newString;

public FormattedWhitespace(SyntaxFormattingOptions options, int lineBreaks, int indentation, string language)
: base(options, language)
public FormattedWhitespace(LineFormattingOptions options, int lineBreaks, int indentation)
: base(options)
{
this.LineBreaks = Math.Max(0, lineBreaks);
this.Spaces = Math.Max(0, indentation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@ namespace Microsoft.CodeAnalysis.Formatting;

internal abstract partial class AbstractTriviaDataFactory
{
protected class ModifiedWhitespace : Whitespace
protected sealed class ModifiedWhitespace : Whitespace
{
private readonly Whitespace? _original;

public ModifiedWhitespace(SyntaxFormattingOptions options, int lineBreaks, int indentation, bool elastic, string language)
: base(options, lineBreaks, indentation, elastic, language)
public ModifiedWhitespace(LineFormattingOptions options, int lineBreaks, int indentation, bool elastic)
: base(options, lineBreaks, indentation, elastic)
{
_original = null;
}

public ModifiedWhitespace(SyntaxFormattingOptions options, Whitespace original, int lineBreaks, int indentation, bool elastic, string language)
: base(options, lineBreaks, indentation, elastic, language)
public ModifiedWhitespace(LineFormattingOptions options, Whitespace original, int lineBreaks, int indentation, bool elastic)
: base(options, lineBreaks, indentation, elastic)
{
Contract.ThrowIfNull(original);
_original = original;
Expand Down Expand Up @@ -83,7 +83,7 @@ public override void Format(
CancellationToken cancellationToken,
int tokenPairIndex = TokenPairIndexNotNeeded)
{
formattingResultApplier(tokenPairIndex, context.TokenStream, new FormattedWhitespace(this.Options, this.LineBreaks, this.Spaces, this.Language));
formattingResultApplier(tokenPairIndex, context.TokenStream, new FormattedWhitespace(this.Options, this.LineBreaks, this.Spaces));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ protected class Whitespace : TriviaData
{
private readonly bool _elastic;

public Whitespace(SyntaxFormattingOptions options, int space, bool elastic, string language)
: this(options, lineBreaks: 0, indentation: space, elastic: elastic, language: language)
public Whitespace(LineFormattingOptions options, int space, bool elastic)
: this(options, lineBreaks: 0, indentation: space, elastic: elastic)
{
Contract.ThrowIfFalse(space >= 0);
}

public Whitespace(SyntaxFormattingOptions options, int lineBreaks, int indentation, bool elastic, string language)
: base(options, language)
public Whitespace(LineFormattingOptions options, int lineBreaks, int indentation, bool elastic)
: base(options)
{
_elastic = elastic;

Expand All @@ -47,11 +47,9 @@ public Whitespace(SyntaxFormattingOptions options, int lineBreaks, int indentati
public override TriviaData WithSpace(int space, FormattingContext context, ChainedFormattingRules formattingRules)
{
if (this.LineBreaks == 0 && this.Spaces == space)
{
return this;
}

return new ModifiedWhitespace(this.Options, this, /*lineBreak*/0, space, elastic: false, language: this.Language);
return new ModifiedWhitespace(this.Options, this, /*lineBreak*/0, space, elastic: false);
}

public override TriviaData WithLine(int line, int indentation, FormattingContext context, ChainedFormattingRules formattingRules, CancellationToken cancellationToken)
Expand All @@ -63,7 +61,7 @@ public override TriviaData WithLine(int line, int indentation, FormattingContext
return this;
}

return new ModifiedWhitespace(this.Options, this, line, indentation, elastic: false, language: this.Language);
return new ModifiedWhitespace(this.Options, this, line, indentation, elastic: false);
}

public override TriviaData WithIndentation(
Expand All @@ -74,7 +72,7 @@ public override TriviaData WithIndentation(
return this;
}

return new ModifiedWhitespace(this.Options, this, this.LineBreaks, indentation, elastic: false, language: this.Language);
return new ModifiedWhitespace(this.Options, this, this.LineBreaks, indentation, elastic: false);
}

public override void Format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Security.Principal;
using System.Threading;
using Microsoft.CodeAnalysis.Diagnostics;
using Roslyn.Utilities;
Expand All @@ -14,23 +17,73 @@ internal abstract partial class AbstractTriviaDataFactory
private const int LineBreakCacheSize = 5;
private const int IndentationLevelCacheSize = 20;

private static readonly Dictionary<LineFormattingOptions, (Whitespace[] spaces, Whitespace[,] whitespaces)> s_optionsToWhitespace = new();
private static Tuple<LineFormattingOptions, (Whitespace[] spaces, Whitespace[,] whitespaces)>? s_lastOptionAndWhitespace;

protected readonly TreeData TreeInfo;
protected readonly SyntaxFormattingOptions Options;
protected readonly LineFormattingOptions Options;

private readonly Whitespace[] _spaces;
private readonly Whitespace?[,] _whitespaces = new Whitespace[LineBreakCacheSize, IndentationLevelCacheSize];
private readonly Whitespace[,] _whitespaces;

protected AbstractTriviaDataFactory(TreeData treeInfo, SyntaxFormattingOptions options)
protected AbstractTriviaDataFactory(TreeData treeInfo, LineFormattingOptions options)
Copy link
Contributor

Choose a reason for hiding this comment

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

TreeData treeInfo

Does this need to be sent in now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll check. But it is exposed as a property on this type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yes, that is used by the derived factories.

{
Contract.ThrowIfNull(treeInfo);

TreeInfo = treeInfo;
Options = options;

_spaces = new Whitespace[SpaceCacheSize];
for (var i = 0; i < SpaceCacheSize; i++)
(_spaces, _whitespaces) = GetSpacesAndWhitespaces(options);
}

private static (Whitespace[] spaces, Whitespace[,] whitespaces) GetSpacesAndWhitespaces(LineFormattingOptions options)
{
// Fast path where we'er asking for the same options as last time
var lastOptionAndWhitespace = s_lastOptionAndWhitespace;
if (lastOptionAndWhitespace?.Item1 == options)
return lastOptionAndWhitespace.Item2;

// Otherwise, get from the dictionary, computing if necessary.
var (spaces, whitespaces) = ComputeAndCacheSpacesAndWhitespaces(options);

// Cache this result for the next time.
s_lastOptionAndWhitespace = Tuple.Create(options, (spaces, whitespaces));
return (spaces, whitespaces);

static (Whitespace[] spaces, Whitespace[,] whitespaces) ComputeAndCacheSpacesAndWhitespaces(LineFormattingOptions options)
{
_spaces[i] = new Whitespace(Options, space: i, elastic: false, language: treeInfo.Root.Language);
// First check if it's already in the cache.
lock (s_optionsToWhitespace)
{
if (s_optionsToWhitespace.TryGetValue(options, out var result))
return result;
}

// If not, compute it.
var spaces = new Whitespace[SpaceCacheSize];
for (var i = 0; i < SpaceCacheSize; i++)
spaces[i] = new Whitespace(options, space: i, elastic: false);

var whitespaces = new Whitespace[LineBreakCacheSize, IndentationLevelCacheSize];
for (var lineIndex = 0; lineIndex < LineBreakCacheSize; lineIndex++)
{
for (var indentationLevel = 0; indentationLevel < IndentationLevelCacheSize; indentationLevel++)
{
var indentation = indentationLevel * options.IndentationSize;
whitespaces[lineIndex, indentationLevel] = new Whitespace(
options, lineBreaks: lineIndex + 1, indentation: indentation, elastic: false);
}
}

// Attempt to store in cache. But defer to any other thread that may have already stored it.
lock (s_optionsToWhitespace)
{
if (s_optionsToWhitespace.TryGetValue(options, out var result))
return result;

s_optionsToWhitespace[options] = (spaces, whitespaces);
return (spaces, whitespaces);
}
}
}

Expand All @@ -41,7 +94,7 @@ protected TriviaData GetSpaceTriviaData(int space, bool elastic = false)
// if result has elastic trivia in them, never use cache
if (elastic)
{
return new Whitespace(this.Options, space, elastic: true, language: this.TreeInfo.Root.Language);
return new Whitespace(this.Options, space, elastic: true);
}

if (space < SpaceCacheSize)
Expand All @@ -50,7 +103,7 @@ protected TriviaData GetSpaceTriviaData(int space, bool elastic = false)
}

// create a new space
return new Whitespace(this.Options, space, elastic: false, language: this.TreeInfo.Root.Language);
return new Whitespace(this.Options, space, elastic: false);
}

protected TriviaData GetWhitespaceTriviaData(int lineBreaks, int indentation, bool useTriviaAsItIs, bool elastic)
Expand All @@ -75,29 +128,13 @@ protected TriviaData GetWhitespaceTriviaData(int lineBreaks, int indentation, bo
if (indentationLevel < IndentationLevelCacheSize)
{
var lineIndex = lineBreaks - 1;
EnsureWhitespaceTriviaInfo(lineIndex, indentationLevel);
return _whitespaces[lineIndex, indentationLevel]!;
}
}

return
useTriviaAsItIs
? new Whitespace(this.Options, lineBreaks, indentation, elastic, language: this.TreeInfo.Root.Language)
: new ModifiedWhitespace(this.Options, lineBreaks, indentation, elastic, language: this.TreeInfo.Root.Language);
}

private void EnsureWhitespaceTriviaInfo(int lineIndex, int indentationLevel)
{
Contract.ThrowIfFalse(lineIndex is >= 0 and < LineBreakCacheSize);
Contract.ThrowIfFalse(indentationLevel >= 0 && indentationLevel < _whitespaces.Length / _whitespaces.Rank);

// set up caches
if (_whitespaces[lineIndex, indentationLevel] == null)
{
var indentation = indentationLevel * Options.IndentationSize;
var triviaInfo = new Whitespace(Options, lineBreaks: lineIndex + 1, indentation: indentation, elastic: false, language: this.TreeInfo.Root.Language);
Interlocked.CompareExchange(ref _whitespaces[lineIndex, indentationLevel], triviaInfo, null);
}
return useTriviaAsItIs
? new Whitespace(this.Options, lineBreaks, indentation, elastic)
: new ModifiedWhitespace(this.Options, lineBreaks, indentation, elastic);
}

public abstract TriviaData CreateLeadingTrivia(SyntaxToken token);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,12 @@ internal abstract class TriviaData
{
protected const int TokenPairIndexNotNeeded = int.MinValue;

private readonly string _language;

protected TriviaData(SyntaxFormattingOptions options, string language)
protected TriviaData(LineFormattingOptions options)
{
Options = options;
_language = language;
}

protected SyntaxFormattingOptions Options { get; }
protected string Language => _language;
Copy link
Member Author

Choose a reason for hiding this comment

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

Never used. Removed.

protected LineFormattingOptions Options { get; }
Copy link
Member Author

Choose a reason for hiding this comment

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

all consumers only need the limited amount of info exposed by LineFormattingOPtions, not the full info in SyntaxFormattingOptions.


public int LineBreaks { get; protected set; }
public int Spaces { get; protected set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
// See the LICENSE file in the project root for more information.

using System.Threading;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.CodeAnalysis.Formatting;

internal abstract class TriviaDataWithList(SyntaxFormattingOptions options, string language) : TriviaData(options, language)
internal abstract class TriviaDataWithList(LineFormattingOptions options) : TriviaData(options)
{
public abstract SyntaxTriviaList GetTriviaList(CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Formatting
Protected ReadOnly _original As String
Protected ReadOnly _newString As String

Public Sub New(options As SyntaxFormattingOptions,
Public Sub New(options As LineFormattingOptions,
original As String,
lineBreaks As Integer,
indentation As Integer,
elastic As Boolean)
MyBase.New(options, lineBreaks, indentation, elastic, LanguageNames.VisualBasic)
MyBase.New(options, lineBreaks, indentation, elastic)

Me._original = original
Me._newString = CreateStringFromState()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Formatting
Private Class ComplexTrivia
Inherits AbstractComplexTrivia

Public Sub New(options As SyntaxFormattingOptions, treeInfo As TreeData, token1 As SyntaxToken, token2 As SyntaxToken)
Public Sub New(options As LineFormattingOptions, treeInfo As TreeData, token1 As SyntaxToken, token2 As SyntaxToken)
MyBase.New(options, treeInfo, token1, token2)
Contract.ThrowIfNull(treeInfo)
End Sub
Expand Down
Loading
Loading