From 069b409dd4eae142106558d0a4f631cb2b7d4938 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 7 May 2020 17:35:41 -0700 Subject: [PATCH] Fix perf issue in LineTrackingStringbuffer.ScanLines. I noticed several hundred ms spent in this method from a customer profile. Primarilly, the method was doing a linear scan of all lines trying to find one that contained the requested position. I changed this to a binary search, but kept/improved the optimization around checking next/previous lines before instigating the search. Note, there was also a bug where the old code did: else if (absoluteIndex > _currentLine.Index && _currentLine.Index + 1 < _lines.Count) but it should have been coparing absoluteIndex with _currentLine.Start \n\nCommit migrated from https://github.com/dotnet/aspnetcore-tooling/commit/32a0f28708cf14f1c35349031fd6f9d33899220b --- .../src/Legacy/LineTrackingStringBuffer.cs | 87 ++++++++++++++----- 1 file changed, 63 insertions(+), 24 deletions(-) diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs index 99879dff15..d99264cb2c 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs @@ -82,25 +82,51 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { TextLine selected = null; - if (_currentLine != null) - { - if (_currentLine.Contains(absoluteIndex)) - { - // This index is on the last read line - selected = _currentLine; - } - else if (absoluteIndex > _currentLine.Index && _currentLine.Index + 1 < _lines.Count) - { - // This index is ahead of the last read line - selected = ScanLines(absoluteIndex, _currentLine.Index); - } - } - - // Have we found a line yet? - if (selected == null) + if (_currentLine == null) { // Scan from line 0 - selected = ScanLines(absoluteIndex, 0); + selected = ScanLines(absoluteIndex, 0, _lines.Count); + } + else if (absoluteIndex >= _currentLine.End) + { + if (_currentLine.Index + 1 < _lines.Count) + { + // This index is after the last read line + var nextLine = _lines[_currentLine.Index + 1]; + + // Optimization to not search if it's the common case where the line after _currentLine is being requested. + if (nextLine.Contains(absoluteIndex)) + { + selected = nextLine; + } + else + { + selected = ScanLines(absoluteIndex, _currentLine.Index, _lines.Count); + } + } + } + else if (absoluteIndex < _currentLine.Start) + { + if (_currentLine.Index > 0) + { + // This index is before the last read line + var prevLine = _lines[_currentLine.Index - 1]; + + // Optimization to not search if it's the common case where the line before _currentLine is being requested. + if (prevLine.Contains(absoluteIndex)) + { + selected = prevLine; + } + else + { + selected = ScanLines(absoluteIndex, 0, _currentLine.Index); + } + } + } + else + { + // This index is on the last read line + selected = _currentLine; } Debug.Assert(selected == null || selected.Contains(absoluteIndex)); @@ -108,18 +134,31 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return selected; } - private TextLine ScanLines(int absoluteIndex, int startPos) + private TextLine ScanLines(int absoluteIndex, int startLineIndex, int endLineIndex) { - for (int i = 0; i < _lines.Count; i++) - { - var idx = (i + startPos) % _lines.Count; - Debug.Assert(idx >= 0 && idx < _lines.Count); + // binary search for the line containing absoluteIndex + var lowIndex = startLineIndex; + var highIndex = _lines.Count; - if (_lines[idx].Contains(absoluteIndex)) + while (lowIndex != highIndex) + { + var midIndex = (lowIndex + highIndex) / 2; + var midLine = _lines[midIndex]; + + if (absoluteIndex >= midLine.End) { - return _lines[idx]; + lowIndex = midIndex + 1; + } + else if (absoluteIndex < midLine.Start) + { + highIndex = midIndex; + } + else + { + return midLine; } } + return null; }