Polish hash function of executor cache key #29556
Conversation
|
Thanks for your contribution! |
| */ | ||
| hash_combine(&seed, prog_desc); | ||
| for (size_t i = 0; i < prog_desc->Size(); ++i) { | ||
| hash_combine(&seed, &prog_desc->Block(i)); |
There was a problem hiding this comment.
hash_combine(&seed, &prog_desc->Block(i));
It seems to use a pointer of a block_desc but not a BlockDesc object to compute hash value. Is this as expected ?
There was a problem hiding this comment.
Yes, prog_desc->Block(i) returns BlockDesc, it also hold a Proto::Message. It's not recommended to hash a Message object from Community discussion.
It's not a very good idea to use proto message as keys because they are not true value types. With the presence of unknown fields, two equal messages in one binary might be treated as different by another, and therefore two different binaries might get two different unordered_map<> even given the exactly same input data (if the hash function is implemented using MessageDifferencer). It's probably better to write your own hash function that has your desired behavior rather than replying on a generic one.
And Block(i) is creted by new，it will not lead to hash conflict while considering both address of ProgramDesc and its BlockDesc (s) and inner Op_size.
* Add more value to calculate hash key * fix size_t * polish code
PR types
Bug fixes
PR changes
OPs
Describe
Polish hash function of executor cache key
BUG描述：
在CI执行部分动转静单测时，存在一定几率的随机挂，retry之后正常。
原因：
之前使用了
ProgramDesc *指针作为了 executor_cache内在unordered_map的 Key。可能存在两个单测用例中的ProgramDesc指针地址一样的情况，导致执行Program A时，命中了Program B的缓存Context，因此报错。修复：
在hash函数计算中，增加更多的Program字段以避免hash冲突。
移除了指针作为map的key，选择hash_value作为key，保持外层接口
std::pair<program_desc*, bool>不变