Conversation
donJoseLuis
left a comment
There was a problem hiding this comment.
Cool stuff. The only thing to think about is if in a disposable type, you wish to check if instances have already been disposed prior to executing public method bodies.
483dd9e to
2bd3656
Compare
ladipro
left a comment
There was a problem hiding this comment.
Thank you @donJoseLuis. I have addressed your feedback and updated the PR.
|
I feel like there was a rumor of eventually replacing Expander entirely, but I might be misremembering. |
benvillalobos
left a comment
There was a problem hiding this comment.
Looks great! Did you get a sense of how many properties were being expanded that only had a single object?
2bd3656 to
6b55daa
Compare
ladipro
left a comment
There was a problem hiding this comment.
I feel like there was a rumor of eventually replacing Expander entirely, but I might be misremembering.
That would be exciting (and scary at the same time). Any pointers to such plans would be welcome!
|
I looked around, and what I found suggested no concrete plans—just an idea: I was probably a little premature here 🙂 |
That's a great question. When building a .NET Core hello world console app, out of 5632 invocations of
|
6b55daa to
86b65b7
Compare
donJoseLuis
left a comment
There was a problem hiding this comment.
thanks for updating to check if instances are disposed in public members.
…urn null (#6414) Fixes #6413 ### Context This is a regression introduced in #6128. MSBuild crashes when evaluating a project where a property function returns null and its result is concatenated with another non-empty value. ### Changes Made Add a null check. ### Testing Fixed and extended the relevant test case.
.NET SDK 5.0.300 ships with MSBuild 16.10, which optimizes away the
implicit expansion of quoted property lists [*]:
FSC : warning FS0203: Invalid warning number '3218 3390'
Fortunately this old (non-portable) workaround still works:
dotnet/sdk#8792 (comment)
----
[*] dotnet/msbuild#6128
Fixes #6063
Context
Property expansion has been identified as one of the hot spots in project evaluation and
ExpandPropertiesLeaveEscapedaloneaccounts for almost 20% of overall evaluation time of simple projects.
Changes Made
Several optimizations have been made to the calltree under
ExpandPropertiesLeaveEscaped. Notably:strchr-like function has been replaced with a simple call toString.IndexOf.ifs have been replaced with aswitch.List<object> resultshas been eliminated.The combined performance win is 10% in
ExpandPropertiesLeaveEscaped, so close to 2% for evaluation overall.Testing
Expander is well covered by existing unit tests.
Notes
Please review commit by commit for easier to read diffs.