advisedy commented on code in PR #3562:
URL: https://github.com/apache/kvrocks/pull/3562#discussion_r3639727148
##########
src/search/hnsw_indexer.cc:
##########
@@ -173,14 +172,15 @@ StatusOr<double> ComputeSimilarity(const VectorItem&
left, const VectorItem& rig
}
HnswIndex::HnswIndex(const SearchKey& search_key, HnswVectorFieldMetadata*
vector, engine::Storage* storage,
- std::random_device::result_type seed)
+ std::optional<std::random_device::result_type> seed)
: search_key(search_key),
metadata(vector),
storage(storage),
- generator(std::mt19937(seed)),
+ seed(seed),
m_level_normalization_factor(1.0 / std::log(metadata->m)) {}
uint16_t HnswIndex::RandomizeLayer() {
+ static thread_local std::mt19937 generator = [this] { return
std::mt19937(seed.value_or(std::random_device()())); }();
Review Comment:
Here is my origin words:
确实感觉seed有点奇怪在这里，但是generator得根据seed去生成出来，不如说这里seed，generator，和我们期望共享的逻辑有点混乱，我的理解有两种思路，第一种思路就是我们考虑对于seed一样的进行共享，不一样的就是各自持有，这种方法用static
+ thread_local + map 应该能实现出来？
但我感觉有点麻烦。第二种思路就是或许可以考虑直接把seed删掉，使用std::random_device，这样的话，static thread_local
std::mt19937 generator; 放到类里面，不再是局部了，这样初始化也不会依赖某个实例。
想知道一下，你觉得我们应该怎么做会比较好呢？如果你想要第一种的思路，我再去思考具体该怎么实现会比较好。如果是第二种的话，我可以现在就去实现出来代码。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
