-
Notifications
You must be signed in to change notification settings - Fork 409
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
Speed up ExpressionKey's comparison. #2129
Conversation
@@ -145,19 +146,24 @@ public static ExpressionKey CreateFromPreset(ExpressionPreset preset) | |||
|
|||
public override string ToString() | |||
{ | |||
return _id.Replace(UnknownPresetPrefix, ""); | |||
return Name; |
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.
The behaviour has not changed.
|
||
if (Preset != other.Preset) return false; | ||
if (Preset != ExpressionPreset.custom) return true; | ||
return Name == other.Name; |
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.
Fast comparison by early pruning via _hashCode
and
Accurate equality comparison when _hashCode
s are collided.
m_clipMap = expressions.Clips.ToDictionary( | ||
x => expressions.CreateKey(x.Clip), | ||
x => x.Clip, | ||
ExpressionKey.Comparer |
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.
Pass IEqualityComparer<ExpressionKey>
// { | ||
// return this.Equals(CreateFromClip(clip)); | ||
// } | ||
|
||
public int CompareTo(ExpressionKey other) |
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.
実は CompareTo
バグってね?と気づいたけど、いったんここでは破壊的変更はしないということでスルー
@@ -192,5 +193,20 @@ public SubAssetKey SubAssetKey | |||
return new SubAssetKey(typeof(VRM10Expression), this.ToString()); | |||
} | |||
} | |||
|
|||
public static IEqualityComparer<ExpressionKey> Comparer { get; } = new EqualityComparer(); |
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.
IEqualityComparer<ExpressionKey>
はユーザにも公開されていると嬉しいので public で公開する
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.
👍
|
||
namespace UniVRM10.Test | ||
{ | ||
public sealed class ExpressionKeyTests |
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.
無かったのでテスト追加
毎フレーム呼び出される
Vrm10RuntimeExpression
中の処理でExpressionKey
の Dictionary Key としての比較処理がそこそこ負荷が高い。これを public な挙動を変更せずに改善する。
13 % くらいは高速化した、かも。