-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add dedicated bit for indicating if a node itself has annotations (versus its children). #72104
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
Changes from all commits
ac0bc70
a23ad59
cf2863c
47ba615
35b0818
043a670
2fd71b7
e88df21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,7 +104,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation | |
| if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); | ||
| } | ||
|
|
||
| this.flags |= NodeFlags.ContainsAnnotations; | ||
| this.flags |= (NodeFlags.HasAnnotationsDirectly | NodeFlags.ContainsAnnotations); | ||
| s_annotationsTable.Add(this, annotations); | ||
| } | ||
| } | ||
|
|
@@ -119,7 +119,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation | |
| if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); | ||
| } | ||
|
|
||
| this.flags |= NodeFlags.ContainsAnnotations; | ||
| this.flags |= (NodeFlags.HasAnnotationsDirectly | NodeFlags.ContainsAnnotations); | ||
| s_annotationsTable.Add(this, annotations); | ||
| } | ||
| } | ||
|
|
@@ -260,19 +260,39 @@ public virtual int FindSlotIndexContainingOffset(int offset) | |
| internal enum NodeFlags : ushort | ||
| { | ||
| None = 0, | ||
| ContainsDiagnostics = 1 << 0, | ||
| ContainsStructuredTrivia = 1 << 1, | ||
| ContainsDirectives = 1 << 2, | ||
| ContainsSkippedText = 1 << 3, | ||
| ContainsAnnotations = 1 << 4, | ||
| IsNotMissing = 1 << 5, | ||
| ContainsAttributes = 1 << 6, | ||
|
|
||
| FactoryContextIsInAsync = 1 << 7, | ||
| FactoryContextIsInQuery = 1 << 8, | ||
| /// <summary> | ||
| /// If this node is missing or not. We use a non-zero value for the not-missing case so that this value | ||
| /// automatically merges upwards when building parent nodes. In other words, once we have one node that is | ||
| /// not-missing, all nodes above it are definitely not-missing as well. | ||
| /// </summary> | ||
| IsNotMissing = 1 << 0, | ||
| /// <summary> | ||
| /// If this node directly has annotations (not its descendants). <see cref="ContainsAnnotations"/> can be | ||
| /// used to determine if a node or any of its descendants has annotations. | ||
| /// </summary> | ||
| HasAnnotationsDirectly = 1 << 1, | ||
|
|
||
| FactoryContextIsInAsync = 1 << 2, | ||
| FactoryContextIsInQuery = 1 << 3, | ||
| FactoryContextIsInIterator = FactoryContextIsInQuery, // VB does not use "InQuery", but uses "InIterator" instead | ||
|
|
||
| InheritMask = ContainsDiagnostics | ContainsStructuredTrivia | ContainsDirectives | ContainsSkippedText | ContainsAnnotations | ContainsAttributes | IsNotMissing, | ||
| // Flags that are inherited upwards when building parent nodes. They should all start with "Contains" to | ||
| // indicate that the information could be found on it or anywhere in its children. | ||
|
|
||
| /// <summary> | ||
| /// If this node, or any of its descendants has annotations attached to them. | ||
| /// </summary> | ||
| ContainsAnnotations = 1 << 4, | ||
| /// <summary> | ||
| /// If this node, or any of its descendants has attributes attached to it. | ||
| /// </summary> | ||
| ContainsAttributes = 1 << 5, | ||
| ContainsDiagnostics = 1 << 6, | ||
| ContainsDirectives = 1 << 7, | ||
| ContainsSkippedText = 1 << 8, | ||
| ContainsStructuredTrivia = 1 << 9, | ||
|
|
||
| InheritMask = IsNotMissing | ContainsAnnotations | ContainsAttributes | ContainsDiagnostics | ContainsDirectives | ContainsSkippedText | ContainsStructuredTrivia, | ||
| } | ||
|
|
||
| internal NodeFlags Flags | ||
|
|
@@ -370,6 +390,15 @@ public bool ContainsAnnotations | |
| return (this.flags & NodeFlags.ContainsAnnotations) != 0; | ||
| } | ||
| } | ||
|
|
||
| public bool HasAnnotationsDirectly | ||
| { | ||
| get | ||
| { | ||
| return (this.flags & NodeFlags.HasAnnotationsDirectly) != 0; | ||
| } | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region Spans | ||
|
|
@@ -541,17 +570,15 @@ private static IEnumerable<SyntaxAnnotation> GetAnnotationsSlow(SyntaxAnnotation | |
|
|
||
| public SyntaxAnnotation[] GetAnnotations() | ||
| { | ||
| if (this.ContainsAnnotations) | ||
| { | ||
| SyntaxAnnotation[]? annotations; | ||
| if (s_annotationsTable.TryGetValue(this, out annotations)) | ||
| { | ||
| System.Diagnostics.Debug.Assert(annotations.Length != 0, "we should return nonempty annotations or NoAnnotations"); | ||
| return annotations; | ||
| } | ||
| } | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the meat of the change. In the past, we'd end up lookign at nodes that didn't actually have annotations on htem, because they had a child somewhere deep under them with annotations. This was very expensive as CWT lookups are not cheap. For tree rewrites, when rewriting nodes , we look at the old node to copy annotations over. And this meant almost always doing this work just if a deep child token had an annotation. now we only need to do the work on nodes that actually have annotations on them themselves. |
||
| if (!this.HasAnnotationsDirectly) | ||
| return s_noAnnotations; | ||
|
|
||
| return s_noAnnotations; | ||
| var found = s_annotationsTable.TryGetValue(this, out var annotations); | ||
| Debug.Assert(found, "We must be able to find annotations since we had the bit set on ourselves"); | ||
| Debug.Assert(annotations != null, "annotations should not be null"); | ||
| Debug.Assert(annotations != s_noAnnotations, "annotations should not be s_noAnnotations"); | ||
| Debug.Assert(annotations.Length != 0, "annotations should be non-empty"); | ||
| return annotations; | ||
| } | ||
|
|
||
| internal abstract GreenNode SetAnnotations(SyntaxAnnotation[]? annotations); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.