Lazy doc comments for signature help - #458
Conversation
Cache the result of the most recent call to DocumentationComment.FromXmlFragment.
Avoid parsing XML to determine completion list type if the text doesn't contain the word "completionlist"
Realize the Documentation for signature help items lazily.
|
@rchande, please take a look. |
There was a problem hiding this comment.
any reason it has to be created ToArray()? foreach seems would just work
There was a problem hiding this comment.
As the note says, I want to avoid enumerating more than once. If I used the IEnumerable, there would be one for the ".Count > 0" check (I would call the .Any() extension method), then there would be two more to call AddRange and Append on lines 202-203.
This is subtle and one of the things I was looking for feedback on. The types I changed are carefully selected to avoid early evaluation by switching to an iterator and threading IEnumerable through to this line. The downside is that, if you enumerate more than once, you incur a 2nd parsing cost.
Now, this could probably be fixed by using a custom iterator type. One that is lazy (doesn't do any work until you call GetEnumerator), but fast for subsequent calls.
|
👍 |
There was a problem hiding this comment.
Can _parameterIndex >= _parameters.Length ever be true?
There was a problem hiding this comment.
If it can, I'm prepared to take the IndexOutOfRangeException. I can add validation in the constructor.
…for Documentation in SignatureHelp and thread that through the entire sig-help implementation.
|
@pharring Ok, i hate to be that guy. But 19 allocations/instances per character sounds like literally nothing. I'm guessing that's orders of magnitude less than the number of objects created by the rest of the system during typing. I'm all for efficiency and cutting down on things. But normally in areas where there's a lot created, such that the savings is actually measurably better. But avoiding lazys just to save a literal handful of allocations seems... excessive. Just my 2c though :) |
|
👍 |
dpoeschl
added
Area-IDE
Tenet-Performance
|
@CyrusNajmabadi Well, since you gave me the option, I'll just make this change and let you fix up Typescript. Will send email too. |
Use compact property getter syntax. Added range checking to Signature's constructor.
Lazy doc comments for signature help Made the retrieval and parsing of XML documentation comments for signature help items lazy. This saves allocations and CPU time for operations such as typing within methods like Console.WriteLine (which has 19 overloads) Also, cache the result of the last XML fragment parse (since we often ask to parse the same fragment over and over)
Verify content before iteration counts
7 participants
This change makes XML documentation comment lookup and parsing for signature help lazy.