Get rid of LineTrackingStringBuffer class and instead use the line in… #22903
Conversation
…formation provided by RazorSourceDocument. This additionally gets rid of an extra whole buffer allocation in the ParserContext. The most complex bit of the change is around avoiding TextLineCollection.GetLocation. Overall, I'm seeing a pretty big win here, about 35% less time spent in RazorSyntaxTree.Parse for the typing scenario I was doing in a very large file.
ghost
added
the
area-mvc
| break; | ||
|
|
| { | ||
| if (source == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(source)); | ||
| } | ||
|
|
||
| _buffer = new LineTrackingStringBuffer(source, filePath); | ||
| _sourceDocument = source; | ||
| _cachedLineInfo = (new TextSpan(0, _sourceDocument.Lines.GetLineLength(0)), 0); |
There was a problem hiding this comment.
This is confusing to me. Why cache the very first line and special case that?
There was a problem hiding this comment.
It simplified the code so it didnt need to check against uninitialized. Also, callers tend to start at the first line.
There was a problem hiding this comment.
This assumes that RazorSourceDocument will have atleast one line right? Is it safe to make that assumption?
There was a problem hiding this comment.
On empty documents, _sourceDocument.Lines.Count == 1. See
In reply to: 440388134 [](ancestors = 440388134)
|
|
||
| public SeekableTextReader(string source, string filePath) : this(source.ToCharArray(), filePath) { } | ||
| public SeekableTextReader(string source, string filePath) : this(new StringSourceDocument(source, Encoding.UTF8, new RazorSourceDocumentProperties(filePath, relativePath: null))) { } |
There was a problem hiding this comment.
Where is this constructor currently being used?
There was a problem hiding this comment.
Two places from test code and once from LanguageCharacteristics.TokenizeString which is called from several places in product code. Perhaps UTF8 isn't appropriate here, maybe Encoding.Unicode?
In reply to: 440378902 [](ancestors = 440378902)
There was a problem hiding this comment.
Spoke offline. UTF8 is fine as we do the same thing in most other places as well.
| { | ||
| if (source == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(source)); | ||
| } | ||
|
|
||
| _buffer = new LineTrackingStringBuffer(source, filePath); | ||
| _sourceDocument = source; | ||
| _cachedLineInfo = (new TextSpan(0, _sourceDocument.Lines.GetLineLength(0)), 0); |
There was a problem hiding this comment.
This assumes that RazorSourceDocument will have atleast one line right? Is it safe to make that assumption?
| if (_position >= _cachedLineInfo.Span.End) | ||
| { | ||
| // Try to avoid the GetLocation call by checking if the next line contains the position | ||
| int nextLineIndex = _cachedLineInfo.LineIndex + 1; |
There was a problem hiding this comment.
| int nextLineIndex = _cachedLineInfo.LineIndex + 1; | |
| var nextLineIndex = _cachedLineInfo.LineIndex + 1; |
Similarly in other places as well
…formation provided by RazorSourceDocument.
This additionally gets rid of an extra whole buffer allocation in the ParserContext. The most complex bit of the change is around avoiding TextLineCollection.GetLocation.
Overall, I'm seeing a pretty big win here, about 35% less time spent in RazorSyntaxTree.Parse for the typing scenario I was doing in a very large file.