Optimize TypeRegistry
#436
Conversation
| public void EnsureNotLocked() | ||
| { | ||
|
|
||
| /// <inheritdoc/> | ||
| public bool IsLocked { [DebuggerStepThrough] get; private set; } | ||
|
|
||
| /// <summary> | ||
| /// Ensures the object is not locked (see <see cref="ILockable.Lock()"/>) yet. | ||
| /// </summary> | ||
| /// <exception cref="InstanceIsLockedException">The instance is locked.</exception> | ||
| public void EnsureNotLocked() | ||
| { | ||
| if (IsLocked) { | ||
| throw new InstanceIsLockedException(Strings.ExInstanceIsLocked); | ||
| } | ||
| if (IsLocked) { | ||
| ThrowInstanceIsLockedException(); |
There was a problem hiding this comment.
It doesn't look like necessary change. The method is small enough and encapsulates Exception throwing inside. Most probably there is no sense of splitting it into 2 even smaller.
There was a problem hiding this comment.
Any throw code is big actually
MS also uses this ThrowHelper pattern over dotnet codebase
See https://dunnhq.com/posts/2022/throw-helper/
|
|
||
| result.ValueColumns.AddRange(valueColumns); | ||
| result.SelectColumns = columnMap; | ||
| result.SelectColumns = columnMap.ToArray(); |
There was a problem hiding this comment.
Because Arrays are more lightweight than List<>
Index building is one-time operation. It is better to store the most efficient data structure
| var valueBuilder = new ValueStringBuilder(expectedLength); | ||
| valueBuilder.Append(BatchBegin); | ||
| + statements.Sum(static statement => statement.Length); | ||
| StringBuilder sb = new(BatchBegin, expectedLength); |
There was a problem hiding this comment.
This is missing a comment why ValueStringBuilder is not suitable.
There was a problem hiding this comment.
To avoid valueBuilder.Append(actualStatement.ToString());
We cannot append ReadOnlySpan<> to ref struct ValueStringBuilder
So I decided to return back to StringBuilder to save an allocation inside loop
Also:
ConvertingEnumerator,ConvertingEnumerableStructureExpression,StructureFieldExpressionIndexInfo: put array into it instead of ListsEnsureNotLocked()- don't pollute CPU cache bythrowstatement