Skip to content

Commit 5478155

Browse files
hvitvedhvitved
authored
Merge pull request #615 from calumgrant/cs/extractor-caching
C# extractor: Improve performance by changing the caching
2 parents a600353 + d73b28e commit 5478155

File tree

20 files changed

+173
-139
lines changed

20 files changed

+173
-139
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public override void Populate()
5858
}
5959
else
6060
{
61-
Context.ModelError(symbol, "Undhandled accessor kind");
61+
Context.ModelError(symbol, "Unhandled accessor kind");
6262
return;
6363
}
6464

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Constructor.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public override IId Id
118118
if (symbol.IsStatic) tb.Append("static");
119119
tb.Append(ContainingType);
120120
AddParametersToId(Context, tb, symbol);
121-
tb.Append("; constructor");
121+
tb.Append(";constructor");
122122
});
123123
}
124124
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Event.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ public override void Populate()
3030
Context.Emit(Tuples.events(this, symbol.GetName(), ContainingType, type.TypeRef, Create(Context, symbol.OriginalDefinition)));
3131

3232
var adder = symbol.AddMethod;
33-
if (adder != null)
33+
var remover = symbol.RemoveMethod;
34+
35+
if (!(adder is null))
3436
EventAccessor.Create(Context, adder);
3537

36-
var remover = symbol.RemoveMethod;
37-
if (remover != null)
38+
if (!(remover is null))
3839
EventAccessor.Create(Context, remover);
3940

4041
ExtractModifiers();

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs‎

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -21,45 +21,21 @@ public override void Populate()
2121
var getter = symbol.GetMethod;
2222
var setter = symbol.SetMethod;
2323

24-
if (getter == null && setter == null)
24+
if (getter is null && setter is null)
2525
Context.ModelError(symbol, "No indexer accessor defined");
2626

27-
if (getter != null)
28-
{
29-
Getter = Accessor.Create(Context, getter);
30-
}
27+
if (!(getter is null))
28+
Method.Create(Context, getter);
3129

32-
if (setter != null)
33-
{
34-
Setter = Accessor.Create(Context, setter);
35-
}
30+
if (!(setter is null))
31+
Method.Create(Context, setter);
3632

3733
for (var i = 0; i < symbol.Parameters.Length; ++i)
3834
{
3935
var original = Parameter.Create(Context, symbol.OriginalDefinition.Parameters[i], OriginalDefinition);
4036
Parameter.Create(Context, symbol.Parameters[i], this, original);
4137
}
4238

43-
if (getter != null)
44-
{
45-
Getter = Accessor.Create(Context, getter);
46-
Context.Emit(Tuples.accessors(Getter, 1, getter.Name, this, Getter.OriginalDefinition));
47-
48-
Context.Emit(Tuples.accessor_location(Getter, Getter.Location));
49-
Getter.Overrides();
50-
Getter.ExtractModifiers();
51-
}
52-
53-
if (setter != null)
54-
{
55-
Setter = Accessor.Create(Context, setter);
56-
Context.Emit(Tuples.accessors(Setter, 2, setter.Name, this, Setter.OriginalDefinition));
57-
58-
Context.Emit(Tuples.accessor_location(Setter, Setter.Location));
59-
Setter.Overrides();
60-
Setter.ExtractModifiers();
61-
}
62-
6339
if (IsSourceDeclaration)
6440
{
6541
var expressionBody = ExpressionBody;

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.cs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,21 +256,28 @@ public static Method Create(Context cx, IMethodSymbol methodDecl)
256256
{
257257
if (methodDecl == null) return null;
258258

259-
switch (methodDecl.MethodKind)
259+
var methodKind = methodDecl.MethodKind;
260+
261+
if(methodKind == MethodKind.ExplicitInterfaceImplementation)
262+
{
263+
// Retrieve the original method kind
264+
methodKind = methodDecl.ExplicitInterfaceImplementations.Select(m => m.MethodKind).FirstOrDefault();
265+
}
266+
267+
switch (methodKind)
260268
{
261269
case MethodKind.StaticConstructor:
262270
case MethodKind.Constructor:
263271
return Constructor.Create(cx, methodDecl);
264272
case MethodKind.ReducedExtension:
265273
case MethodKind.Ordinary:
266274
case MethodKind.DelegateInvoke:
267-
case MethodKind.ExplicitInterfaceImplementation:
268275
return OrdinaryMethod.Create(cx, methodDecl);
269276
case MethodKind.Destructor:
270277
return Destructor.Create(cx, methodDecl);
271278
case MethodKind.PropertyGet:
272279
case MethodKind.PropertySet:
273-
return Accessor.Create(cx, methodDecl);
280+
return methodDecl.AssociatedSymbol is null ? OrdinaryMethod.Create(cx, methodDecl) : (Method)Accessor.Create(cx, methodDecl);
274281
case MethodKind.EventAdd:
275282
case MethodKind.EventRemove:
276283
return EventAccessor.Create(cx, methodDecl);

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace Semmle.Extraction.CSharp.Entities
44
{
5-
class Namespace : CachedEntity<INamespaceSymbol>
5+
sealed class Namespace : CachedEntity<INamespaceSymbol>
66
{
77
Namespace(Context cx, INamespaceSymbol init)
88
: base(cx, init) { }
@@ -31,7 +31,7 @@ public override IId Id
3131
}
3232
}
3333

34-
public static Namespace Create(Context cx, INamespaceSymbol ns) => NamespaceFactory.Instance.CreateEntity(cx, ns);
34+
public static Namespace Create(Context cx, INamespaceSymbol ns) => NamespaceFactory.Instance.CreateEntity2(cx, ns);
3535

3636
class NamespaceFactory : ICachedEntityFactory<INamespaceSymbol, Namespace>
3737
{
@@ -41,5 +41,14 @@ class NamespaceFactory : ICachedEntityFactory<INamespaceSymbol, Namespace>
4141
}
4242

4343
public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.NoLabel;
44+
45+
public override int GetHashCode() => QualifiedName.GetHashCode();
46+
47+
string QualifiedName => symbol.ToDisplayString();
48+
49+
public override bool Equals(object obj)
50+
{
51+
return obj is Namespace ns && QualifiedName == ns.QualifiedName;
52+
}
4453
}
4554
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ public override IId Id
2424
}
2525
}
2626

27-
protected Accessor Getter { get; set; }
28-
protected Accessor Setter { get; set; }
29-
3027
public override void Populate()
3128
{
3229
ExtractAttributes();
@@ -37,12 +34,13 @@ public override void Populate()
3734
Context.Emit(Tuples.properties(this, symbol.GetName(), ContainingType, type.TypeRef, Create(Context, symbol.OriginalDefinition)));
3835

3936
var getter = symbol.GetMethod;
40-
if (getter != null)
41-
Getter = Accessor.Create(Context, getter);
42-
4337
var setter = symbol.SetMethod;
44-
if (setter != null)
45-
Setter = Accessor.Create(Context, setter);
38+
39+
if (!(getter is null))
40+
Method.Create(Context, getter);
41+
42+
if (!(setter is null))
43+
Method.Create(Context, setter);
4644

4745
var declSyntaxReferences = IsSourceDeclaration ?
4846
symbol.DeclaringSyntaxReferences.
@@ -100,7 +98,9 @@ public override Microsoft.CodeAnalysis.Location FullLocation
10098

10199
public static Property Create(Context cx, IPropertySymbol prop)
102100
{
103-
return prop.IsIndexer ? Indexer.Create(cx, prop) : PropertyFactory.Instance.CreateEntity(cx, prop);
101+
bool isIndexer = prop.IsIndexer || prop.ExplicitInterfaceImplementations.Any(e => e.IsIndexer);
102+
103+
return isIndexer ? Indexer.Create(cx, prop) : PropertyFactory.Instance.CreateEntity(cx, prop);
104104
}
105105

106106
public void VisitDeclaration(Context cx, PropertyDeclarationSyntax p)

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public NamedTypeRef(Context cx, INamedTypeSymbol symbol) : base(cx, symbol)
174174
referencedType = Type.Create(cx, symbol);
175175
}
176176

177-
public static NamedTypeRef Create(Context cx, INamedTypeSymbol type) => NamedTypeRefFactory.Instance.CreateEntity(cx, type);
177+
public static NamedTypeRef Create(Context cx, INamedTypeSymbol type) => NamedTypeRefFactory.Instance.CreateEntity2(cx, type);
178178

179179
class NamedTypeRefFactory : ICachedEntityFactory<INamedTypeSymbol, NamedTypeRef>
180180
{

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ protected void ExtractType()
157157
var param = invokeMethod.Parameters[i];
158158
var originalParam = invokeMethod.OriginalDefinition.Parameters[i];
159159
var originalParamEntity = Equals(param, originalParam) ? null :
160-
DelegateTypeParameter.Create(Context, originalParam, Create(Context, ((INamedTypeSymbol)symbol).ConstructedFrom));
160+
DelegateTypeParameter.Create(Context, originalParam, Create(Context, ((INamedTypeSymbol)symbol).OriginalDefinition));
161161
DelegateTypeParameter.Create(Context, param, this, originalParamEntity);
162162
}
163163

‎csharp/extractor/Semmle.Extraction/Context.cs‎

Lines changed: 83 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -43,28 +43,90 @@ public SemanticModel Model(SyntaxNode node)
4343
int NewId() => TrapWriter.IdCounter++;
4444

4545
/// <summary>
46-
/// Gets the cached label for the given entity, or creates a new
47-
/// (cached) label if it hasn't already been created. The label
48-
/// is set on the supplied <paramref name="entity"/> object.
46+
/// Creates a new entity using the factory.
4947
/// </summary>
50-
/// <returns><code>true</code> iff the label already existed.</returns>
51-
public bool GetOrAddCachedLabel(ICachedEntity entity)
48+
/// <param name="factory">The entity factory.</param>
49+
/// <param name="init">The initializer for the entity.</param>
50+
/// <returns>The new/existing entity.</returns>
51+
public Entity CreateEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
5252
{
53-
var id = GetId(entity);
54-
if (id == null)
55-
throw new InternalError("Attempt to create a null entity for {0}", entity.GetType());
53+
return init == null ? CreateEntity2(factory, init) : CreateNonNullEntity(factory, init);
54+
}
5655

57-
Label existingLabel;
58-
if (labelCache.TryGetValue(id, out existingLabel))
56+
/// <summary>
57+
/// Creates a new entity using the factory.
58+
/// Uses a different cache to <see cref="CreateEntity{Type, Entity}(ICachedEntityFactory{Type, Entity}, Type)"/>,
59+
/// and can store null values.
60+
/// </summary>
61+
/// <param name="factory">The entity factory.</param>
62+
/// <param name="init">The initializer for the entity.</param>
63+
/// <returns>The new/existing entity.</returns>
64+
public Entity CreateEntity2<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
65+
{
66+
using (StackGuard)
5967
{
60-
entity.Label = existingLabel;
61-
return true;
68+
var entity = factory.Create(this, init);
69+
70+
if (entityLabelCache.TryGetValue(entity, out var label))
71+
{
72+
entity.Label = label;
73+
}
74+
else
75+
{
76+
var id = entity.Id;
77+
#if DEBUG_LABELS
78+
CheckEntityHasUniqueLabel(id, entity);
79+
#endif
80+
label = new Label(NewId());
81+
entity.Label = label;
82+
entityLabelCache[entity] = label;
83+
DefineLabel(label, id);
84+
if (entity.NeedsPopulation)
85+
Populate(init as ISymbol, entity);
86+
}
87+
return entity;
6288
}
89+
}
6390

64-
entity.Label = new Label(NewId());
65-
DefineLabel(entity.Label, id);
66-
labelCache[id] = entity.Label;
67-
return false;
91+
#if DEBUG_LABELS
92+
private void CheckEntityHasUniqueLabel(IId id, ICachedEntity entity)
93+
{
94+
if (idLabelCache.TryGetValue(id, out var originalEntity))
95+
{
96+
Extractor.Message(new Message { message = "Label collision for " + id.ToString(), severity = Severity.Warning });
97+
}
98+
else
99+
{
100+
idLabelCache[id] = entity;
101+
}
102+
}
103+
#endif
104+
105+
private Entity CreateNonNullEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
106+
{
107+
if (objectEntityCache.TryGetValue(init, out var cached))
108+
return (Entity)cached;
109+
110+
using (StackGuard)
111+
{
112+
var label = new Label(NewId());
113+
var entity = factory.Create(this, init);
114+
entity.Label = label;
115+
116+
objectEntityCache[init] = entity;
117+
118+
var id = entity.Id;
119+
DefineLabel(label, id);
120+
121+
#if DEBUG_LABELS
122+
CheckEntityHasUniqueLabel(id, entity);
123+
#endif
124+
125+
if (entity.NeedsPopulation)
126+
Populate(init as ISymbol, entity);
127+
128+
return entity;
129+
}
68130
}
69131

70132
/// <summary>
@@ -90,25 +152,6 @@ public bool ExtractGenerics(ICachedEntity entity)
90152
}
91153
}
92154

93-
/// <summary>
94-
/// Gets the ID belonging to cached entity <paramref name="entity"/>.
95-
///
96-
/// The ID itself is also cached, but unlike the label cache (which is used
97-
/// to prevent reextraction/infinite loops), this is a pure performance
98-
/// optimization. Moreover, the label cache is injective, which the ID cache
99-
/// need not be.
100-
/// </summary>
101-
IId GetId(ICachedEntity entity)
102-
{
103-
IId id;
104-
if (!idCache.TryGetValue(entity, out id))
105-
{
106-
id = entity.Id;
107-
idCache[entity] = id;
108-
}
109-
return id;
110-
}
111-
112155
/// <summary>
113156
/// Creates a fresh label with ID "*", and set it on the
114157
/// supplied <paramref name="entity"/> object.
@@ -120,7 +163,11 @@ public void AddFreshLabel(IEntity entity)
120163
entity.Label = label;
121164
}
122165

123-
readonly Dictionary<IId, Label> labelCache = new Dictionary<IId, Label>();
166+
#if DEBUG_LABELS
167+
readonly Dictionary<IId, ICachedEntity> idLabelCache = new Dictionary<IId, ICachedEntity>();
168+
#endif
169+
readonly Dictionary<object, ICachedEntity> objectEntityCache = new Dictionary<object, ICachedEntity>();
170+
readonly Dictionary<ICachedEntity, Label> entityLabelCache = new Dictionary<ICachedEntity, Label>();
124171
readonly HashSet<Label> extractedGenerics = new HashSet<Label>();
125172

126173
public void DefineLabel(IEntity entity)
@@ -134,8 +181,6 @@ void DefineLabel(Label label, IId id)
134181
TrapWriter.Emit(new DefineLabelEmitter(label, id));
135182
}
136183

137-
readonly Dictionary<object, IId> idCache = new Dictionary<object, IId>();
138-
139184
/// <summary>
140185
/// Queue of items to populate later.
141186
/// The only reason for this is so that the call stack does not

0 commit comments

Comments
 (0)