Skip to content
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

[mp] use hash from state in dfs #206

Closed
wants to merge 112 commits into from

Conversation

KiK0S
Copy link
Collaborator

@KiK0S KiK0S commented Feb 17, 2023

No description provided.

for event in self.events.iter() {
let mut h = DefaultHasher::default();
event.hash(&mut h);
hash_events ^= h.finish();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Немного здесь порассуждаю. Нам нужно сделать так, чтобы хэш не зависел от порядка событий в events. Сейчас это сделано с помощью XOR (есть еще подход с суммированием, но непонятно какой из них лучше). У такой реализации есть недостаток, что надо создавать hasher на каждый элемент, что может добавлять заметный оверхед для небольших событий типа таймеров.

Альтернативный подход - хранить события изначально в некотором фиксированном порядке. Тогда можно было бы пользоваться готовой реализацией Hash, без такого оверхеда. Но здесь есть проблема с выбором подходящей структуры данных. BTreeSet не подходит, потому что теоретически у нас может быть несколько полностью совпадающих событий (идентичных сообщений между парой процессов или таймеров с одинаковым именем у одного процесса). И надо еще чтобы эта структура была удобна для работы с ней в стратегии, где надо извлекать произвольные события.

Посколько мы хотим в ближайшее время заменить events на Dependency Resolver, то предлагаю пока оставить вычисление хэша как есть. А после миграции на Resolver еще раз посмотреть, можно ли будет перейти на альтернативный подход.

@osukhoroslov osukhoroslov changed the title use hash from state in dfs [mp] use hash from state in dfs Feb 19, 2023
@AnthonyUdovichenko AnthonyUdovichenko force-pushed the mc-initial branch 2 times, most recently from c17e6f2 to adc7bc2 Compare February 26, 2023 18:15
@KiK0S KiK0S force-pushed the kik0s/hash-in-mc branch from 4f679ab to 3d861bd Compare March 9, 2023 22:21
@KiK0S KiK0S requested a review from osukhoroslov March 10, 2023 17:45
@KiK0S KiK0S force-pushed the kik0s/hash-in-mc branch from edc9d10 to db023c3 Compare March 10, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants