Use char overload in System.Xaml #3998
Conversation
ghost
added
the
ghost
requested review from
| @@ -1996,7 +1996,7 @@ private void Logic_DoAssignmentToParentProperty(ObjectWriterContext ctx) | |||
| if (value is NameFixupToken && parentProperty != XamlLanguage.Items) | |||
| { | |||
| NameFixupToken token = value as NameFixupToken; | |||
| string names = String.Join(",", token.NeededNames.ToArray()); | |||
| string names = String.Join(',', token.NeededNames.ToArray()); | |||
There was a problem hiding this comment.
Curious, are these ToArray calls needed? It seems like NeededNames implements IEnumerable<String>. (Not that it probably matters on this exception oath)
There was a problem hiding this comment.
It would definetly be worth benchmarking but List may have optimization that makes it quick to return a copy of the internal array so I don't know if it is faster than enumerating with IEnumerable. My guess would be that ToArray is faster but uses more memory (It may vary with the number of item).
There was a problem hiding this comment.
List creates a new array and copies elements over, while enumeration uses in-place value-type enumerator. It's a one method call vs. method call every item during enumerating, but also don't forget that the string.Join implementation differs - the array uses unsafe, pre-allocated char buffer while the enumerable uses StringBuilder to avoid double enumeration, and another difference is that changing the collection while being enumerated is illegal and throws.
That said, it probably doesn't matter here as it's an exception path and the number of items in the list would typically be quite small.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thank you for your contribution, @ThomasGoulet73. |
Replace all method call that uses a single char string while there is another method that accepts a char.
Should yield tiny performance benefit on hot paths.