-
Notifications
You must be signed in to change notification settings - Fork 3
Optimize TypeRegistry
#436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
53483b3
c032bce
2730d43
eb091d7
be0b904
983ec61
1969a25
601377e
d512929
f184be4
52a5744
3d5c161
3939992
a62873e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -632,7 +632,7 @@ private IndexInfo BuildJoinIndex(TypeInfo reflectedType, IEnumerable<IndexInfo> | |
| valueColumnMapping.Add((item.i, item.columns)); | ||
| } | ||
|
|
||
| result.ValueColumnsMap = valueColumnMapping; | ||
| result.ValueColumnsMap = valueColumnMapping.ToArray(); | ||
| result.ValueColumns.AddRange(GatherValueColumns(columnsToAdd)); | ||
| result.Name = nameBuilder.BuildIndexName(reflectedType, result); | ||
| result.Group = BuildColumnGroup(result); | ||
|
|
@@ -757,7 +757,7 @@ private IndexInfo BuildViewIndex(TypeInfo reflectedType, IndexInfo indexToApplyV | |
| } | ||
|
|
||
| result.ValueColumns.AddRange(valueColumns); | ||
| result.SelectColumns = columnMap; | ||
| result.SelectColumns = columnMap.ToArray(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is it better than before?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because Arrays are more lightweight than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably it won't be noticeable (several additional bytes for the List<> and probably no difference in data access due to compiler optimizations), but ok, thanks for clarification. |
||
| result.Name = nameBuilder.BuildIndexName(reflectedType, result); | ||
| result.Group = BuildColumnGroup(result); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any
throwcode is big actuallyMS also uses this
ThrowHelperpattern overdotnetcodebaseSee https://dunnhq.com/posts/2022/throw-helper/
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know that. But you just moved
throwto another method, and that's it. No benefit IMHO.Throw helpers behave just like this old implementation of
EnsureNotLocked: they check something and throw.