Skip to content

Commit 133c257

Browse files
ExaniteExanite
authored
[3.0] Improve generator performance (#2567)
* Update ClangSharp.PInvokeGenerator * Update comment on ModifyAllReferencesAsync in TransformHandles * Format files using CSharpier These were written before I installed the CSharpier plugin so they were not formatted. * Remove GetTypeInfo call GetTypeInfo should be unnecessary since it gets the type of an expression. For non-type expressions (eg: `5` is an int), this isn't useful for us. This is arguably an indirect reference to int, but the current Silk generator only cares about modifying direct references. For type expressions (eg: `int` refers to int), GetSymbolInfo will also return the same symbol. * Get the compilation once* *Roslyn already caches this internally. This is mainly for avoiding the async method call. Difference is probably not measurable, but theoretically this is faster. * Compare against _relevantIdentifiers set before calling GetSymbolInfo * Skip using directives * Add ProfilingScope * Add comment on _relevantIdentifiers * Fix field naming convention in ProfilingScope * Optimize document renaming in PrettifyNames Conflict checking was O(n), now it is O(1). * Use normalized relative paths during conflict checking This is to prevent false negatives from mismatched path formats. * Remove NormalizeWhitespace calls (much faster, but breaks generator) (cherry picked from commit 8cf1324) * Fix issue where the syntax tree gets corrupted due to reparsing (cherry picked from commit 2c48fb6) * Edit comment and error message (cherry picked from commit 6b12c69) * Avoid normalizing the syntax node twice when formatting * Avoid rebuilding the _relevantIdentifiers set for every document * Move collections in ResolveConflictsProcessor to be closer to where they are used * Preserve original line ending behavior (cherry picked from commit 66d0daa) * Update SDL to v3.44 from v3.2.4 to fix compile error * Revert change to get the compilation once during ModifyAllReferencesAsync This change caused incorrect behavior since the document ids can come from projects other than the source project (namely the test project). This broken when semantic models were requested for test project documents from the source project compilation. * Revert "Update SDL to v3.44 from v3.2.4 to fix compile error" This reverts commit 00d83bc. * Revert "Update ClangSharp.PInvokeGenerator" This reverts commit 7938a0c. * Revert "Preserve original line ending behavior" This reverts commit 364015d. * Add comment on why UsingDirectiveSyntaxes are skipped
1 parent 9d21e44 commit 133c257

23 files changed

Lines changed: 263 additions & 170 deletions

‎sources/SilkTouch/SilkTouch/Mods/AddApiProfiles.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,7 @@ rewriter.Profile is null
265265
logger.LogDebug("No profile identified for {}", path);
266266
}
267267

268-
ctx.SourceProject = doc.WithSyntaxRoot(
269-
rewriter.Visit(root).NormalizeWhitespace()
270-
).Project;
268+
ctx.SourceProject = doc.WithSyntaxRoot(rewriter.Visit(root)).Project;
271269
rewriter.Profile = null;
272270
}
273271
}

‎sources/SilkTouch/SilkTouch/Mods/AddOpaqueStructs.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,10 @@ public Task ExecuteAsync(IModContext ctx, CancellationToken ct = default)
7272
{
7373
var qualified = name.LastIndexOf('.');
7474
var ns =
75-
qualified != -1
76-
? ModUtils.NamespaceIntoIdentifierName(name.AsSpan()[..qualified])
77-
: _defaultNamespaces.TryGetValue(ctx.JobKey, out var def)
78-
? ModUtils.NamespaceIntoIdentifierName(def)
79-
: null;
75+
qualified != -1 ? ModUtils.NamespaceIntoIdentifierName(name.AsSpan()[..qualified])
76+
: _defaultNamespaces.TryGetValue(ctx.JobKey, out var def)
77+
? ModUtils.NamespaceIntoIdentifierName(def)
78+
: null;
8079
if (ns is null)
8180
{
8281
logger.LogWarning(
@@ -107,8 +106,7 @@ public Task ExecuteAsync(IModContext ctx, CancellationToken ct = default)
107106
)
108107
)
109108
)
110-
)
111-
.NormalizeWhitespace(),
109+
),
112110
filePath: proj.FullPath(fname)
113111
).Project;
114112
}

‎sources/SilkTouch/SilkTouch/Mods/AddVTables.cs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,8 +1454,7 @@ public async Task ExecuteAsync(IModContext ctx, CancellationToken ct = default)
14541454

14551455
rw.Reset();
14561456
proj = doc.WithSyntaxRoot(
1457-
rw.Visit(node)?.NormalizeWhitespace()
1458-
?? throw new InvalidOperationException("Visit returned null")
1457+
rw.Visit(node) ?? throw new InvalidOperationException("Visit returned null")
14591458
).Project;
14601459
if (rw.InterfacePartials.Count == 0)
14611460
{
@@ -1515,11 +1514,7 @@ rw.ClassName is not null
15151514
)
15161515
{
15171516
proj = proj
1518-
?.AddDocument(
1519-
Path.GetFileName(fname),
1520-
root.NormalizeWhitespace(),
1521-
filePath: proj.FullPath(fname)
1522-
)
1517+
?.AddDocument(Path.GetFileName(fname), root, filePath: proj.FullPath(fname))
15231518
.Project;
15241519
}
15251520

‎sources/SilkTouch/SilkTouch/Mods/BakeSourceSets.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ .. ctx
391391
{
392392
proj = proj.AddDocument(
393393
Path.GetFileName(bakedPath),
394-
bakedRoot.NormalizeWhitespace(),
394+
bakedRoot,
395395
// we can forgive the below nulls because RelativePath checks them, and returns null if they're null.
396396
filePath: proj.FullPath(bakedPath)
397397
).Project;

‎sources/SilkTouch/SilkTouch/Mods/ChangeNamespace.cs‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ public Task<List<ResponseFile>> BeforeScrapeAsync(string key, List<ResponseFile>
5959

6060
rsps[i] = rsp with
6161
{
62-
GeneratorConfiguration = rsp.GeneratorConfiguration.ToWrapper() with {
62+
GeneratorConfiguration = rsp.GeneratorConfiguration.ToWrapper() with
63+
{
6364
DefaultNamespace = def,
6465
WithNamespaces = with,
6566
},
@@ -91,7 +92,7 @@ public async Task ExecuteAsync(IModContext ctx, CancellationToken ct = default)
9192
var doc =
9293
proj!.GetDocument(docId) ?? throw new InvalidOperationException("Document missing");
9394
proj = doc.WithSyntaxRoot(
94-
rewriter.Visit(await doc.GetSyntaxRootAsync(ct))?.NormalizeWhitespace()
95+
rewriter.Visit(await doc.GetSyntaxRootAsync(ct))
9596
?? throw new InvalidOperationException("Visit returned null.")
9697
).Project;
9798
}
@@ -116,17 +117,16 @@ public Rewriter(
116117
_usingsToAdd.Clear();
117118
return base.VisitCompilationUnit(node) switch
118119
{
119-
CompilationUnitSyntax syntax
120-
=> syntax.AddUsings(
121-
_usingsToAdd
122-
.Select(x => UsingDirective(ModUtils.NamespaceIntoIdentifierName(x)))
123-
.Where(x =>
124-
syntax.Usings.All(y => x.Name?.ToString() != y.Name?.ToString())
125-
)
126-
.ToArray()
127-
),
120+
CompilationUnitSyntax syntax => syntax.AddUsings(
121+
_usingsToAdd
122+
.Select(x => UsingDirective(ModUtils.NamespaceIntoIdentifierName(x)))
123+
.Where(x =>
124+
syntax.Usings.All(y => x.Name?.ToString() != y.Name?.ToString())
125+
)
126+
.ToArray()
127+
),
128128
{ } ret => ret,
129-
null => null
129+
null => null,
130130
};
131131
}
132132

@@ -140,10 +140,11 @@ CompilationUnitSyntax syntax
140140
}
141141
return base.VisitNamespaceDeclaration(node) switch
142142
{
143-
NamespaceDeclarationSyntax syntax
144-
=> syntax.WithName(ModUtils.NamespaceIntoIdentifierName(newNs)),
143+
NamespaceDeclarationSyntax syntax => syntax.WithName(
144+
ModUtils.NamespaceIntoIdentifierName(newNs)
145+
),
145146
{ } ret => ret,
146-
null => null
147+
null => null,
147148
};
148149
}
149150

@@ -159,10 +160,11 @@ FileScopedNamespaceDeclarationSyntax node
159160
}
160161
return base.VisitFileScopedNamespaceDeclaration(node) switch
161162
{
162-
FileScopedNamespaceDeclarationSyntax syntax
163-
=> syntax.WithName(ModUtils.NamespaceIntoIdentifierName(newNs)),
163+
FileScopedNamespaceDeclarationSyntax syntax => syntax.WithName(
164+
ModUtils.NamespaceIntoIdentifierName(newNs)
165+
),
164166
{ } ret => ret,
165-
null => null
167+
null => null,
166168
};
167169
}
168170
}

‎sources/SilkTouch/SilkTouch/Mods/Common/MSBuildModContext.cs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -308,13 +308,8 @@ private static async Task<string> ToNormalisedStringAsync(
308308
CancellationToken ct = default
309309
)
310310
{
311-
var result = await CodeFormatter.FormatAsync(
312-
root.NormalizeWhitespace().SyntaxTree,
313-
_opts,
314-
ct
315-
);
316-
return !result.CompilationErrors.Any()
317-
? result.Code
318-
: root.NormalizeWhitespace(eol: "\n").ToFullString();
311+
var normalizedRoot = root.NormalizeWhitespace();
312+
var result = await CodeFormatter.FormatAsync(normalizedRoot.SyntaxTree, _opts, ct);
313+
return !result.CompilationErrors.Any() ? result.Code : normalizedRoot.ToFullString();
319314
}
320315
}

‎sources/SilkTouch/SilkTouch/Mods/ExtractHandles.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public override async Task ExecuteAsync(IModContext ctx, CancellationToken ct =
5151
project = project
5252
.AddDocument(
5353
Path.GetFileName(relativePath),
54-
node.NormalizeWhitespace(),
54+
node,
5555
filePath: project.FullPath(relativePath)
5656
)
5757
.Project;

‎sources/SilkTouch/SilkTouch/Mods/ExtractNestedTyping.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public override async Task ExecuteAsync(IModContext ctx, CancellationToken ct =
8686
// This is also where extracted enums are processed.
8787
rewriter.File = fname;
8888
project = doc.WithSyntaxRoot(
89-
rewriter.Visit(node)?.NormalizeWhitespace()
89+
rewriter.Visit(node)
9090
?? throw new InvalidOperationException("Rewriter returned null")
9191
).Project;
9292

@@ -110,8 +110,7 @@ rewriter.Namespace is not null
110110
)
111111
)
112112
: SingletonList<MemberDeclarationSyntax>(newStruct)
113-
)
114-
.NormalizeWhitespace(),
113+
),
115114
filePath: project.FullPath(
116115
$"{fname.AsSpan()[..fname.LastIndexOf('/')]}/{newStruct.Identifier}.gen.cs"
117116
)
@@ -174,8 +173,7 @@ rewriter.Namespace is not null
174173
.WithMembers(SingletonList(typeDecl))
175174
)
176175
: SingletonList(typeDecl)
177-
)
178-
.NormalizeWhitespace(),
176+
),
179177
filePath: project.FullPath($"{dir}/{identifier}.gen.cs")
180178
)
181179
.Project;

‎sources/SilkTouch/SilkTouch/Mods/InterceptNativeFunctions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public async Task ExecuteAsync(IModContext ctx, CancellationToken ct = default)
4848
var doc =
4949
proj!.GetDocument(docId) ?? throw new InvalidOperationException("Document missing");
5050
proj = doc.WithSyntaxRoot(
51-
rewriter.Visit(await doc.GetSyntaxRootAsync(ct))?.NormalizeWhitespace()
51+
rewriter.Visit(await doc.GetSyntaxRootAsync(ct))
5252
?? throw new InvalidOperationException("Visit returned null.")
5353
).Project;
5454
}

‎sources/SilkTouch/SilkTouch/Mods/LocationTransformation/IdentifierRenamingTransformer.cs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,40 @@ public class IdentifierRenamingTransformer : LocationTransformer
1414
{
1515
private ISymbol symbol = null!;
1616

17-
private readonly IReadOnlyDictionary<string, List<(ISymbol Symbol, string NewName)>> newNameLookup;
17+
private readonly IReadOnlyDictionary<
18+
string,
19+
List<(ISymbol Symbol, string NewName)>
20+
> newNameLookup;
1821

1922
/// <summary>
2023
/// Creates a new IdentifierRenamingTransformer.
2124
/// </summary>
2225
/// <param name="newNames">The new names for each symbol</param>
23-
public IdentifierRenamingTransformer(IEnumerable<(ISymbol Symbol, string NewName)> newNames) : this(CreateNameLookup(newNames)) {}
26+
public IdentifierRenamingTransformer(IEnumerable<(ISymbol Symbol, string NewName)> newNames)
27+
: this(CreateNameLookup(newNames)) { }
2428

2529
/// <summary>
2630
/// Creates a new IdentifierRenamingTransformer.
2731
/// </summary>
2832
/// <param name="newNameLookup">The new names for each symbol grouped by symbol name.</param>
29-
public IdentifierRenamingTransformer(IReadOnlyDictionary<string, List<(ISymbol Symbol, string NewName)>> newNameLookup)
33+
public IdentifierRenamingTransformer(
34+
IReadOnlyDictionary<string, List<(ISymbol Symbol, string NewName)>> newNameLookup
35+
)
3036
{
3137
this.newNameLookup = newNameLookup;
3238
}
3339

3440
/// <summary>
3541
/// Creates a name lookup dictionary designed for <see cref="IdentifierRenamingTransformer"/>.
3642
/// </summary>
37-
public static IReadOnlyDictionary<string, List<(ISymbol Symbol, string NewName)>> CreateNameLookup(IEnumerable<(ISymbol Symbol, string NewName)> names)
43+
public static IReadOnlyDictionary<
44+
string,
45+
List<(ISymbol Symbol, string NewName)>
46+
> CreateNameLookup(IEnumerable<(ISymbol Symbol, string NewName)> names)
3847
{
39-
return names.GroupBy(t => t.Symbol.Name).ToDictionary(group => group.Key, group => group.ToList());
48+
return names
49+
.GroupBy(t => t.Symbol.Name)
50+
.ToDictionary(group => group.Key, group => group.ToList());
4051
}
4152

4253
/// <inheritdoc />
@@ -68,7 +79,7 @@ private SyntaxToken GetRenamed(ISymbol symbol, SyntaxToken currentNameIdentifier
6879
return currentNameIdentifier;
6980
}
7081

71-
// ----- Types -----
82+
// ----- Types -----
7283

7384
/// <inheritdoc />
7485
public override SyntaxNode VisitClassDeclaration(ClassDeclarationSyntax node)

0 commit comments

Comments
 (0)