use murmur hash for class name hashing #1240
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
from
b7ad2cb to
6e9dae7
Compare
apolyakov
changed the title
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
2 times, most recently
from
2e61c71 to
31927b7
Compare
astrophysik
reviewed
Feb 24, 2025
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
from
31927b7 to
104ffe0
Compare
mkornaukhov03
requested changes
Feb 25, 2025
common/algorithms/hashes.h
Outdated
Comment on lines
50
to
54
| explicit range_hasher(Hasher hasher = Hasher()) : | ||
| Hasher(std::move(hasher)) { | ||
| } | ||
| explicit range_hasher(Hasher hasher = Hasher()) | ||
| : Hasher(std::move(hasher)) {} |
There was a problem hiding this comment.
Extra diff here and in other places in this file
common/common.cmake
Outdated
| @@ -9,6 +9,7 @@ prepend(COMMON_MAIN_SOURCES ${COMMON_DIR}/ | |||
|
|
|||
| sha1.cpp | |||
| allocators/freelist.cpp | |||
| algorithms/murmur-hash.cpp | |||
There was a problem hiding this comment.
Need more space, in compiler.cmake too.
There was a problem hiding this comment.
It looks nicely indented in both github editor and my IDE
There was a problem hiding this comment.
It's tab, we use spaces instead
runtime/datetime/datetime.h
Outdated
Comment on lines
27
to
28
| int get_hash() const noexcept final { | ||
| return 2141635158; | ||
| std::string_view name_view{get_class()}; | ||
| return static_cast<int32_t>(vk::murmur_hash<uint32_t>(name_view.data(), name_view.size())); |
There was a problem hiding this comment.
Can't murmurhash be defined as constexpr?
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
from
7aad41e to
609c311
Compare
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
from
1b9661f to
75caf22
Compare
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
from
983e1d0 to
d4e1f27
Compare
This reverts commit 94286be.
apolyakov
force-pushed
the
apolyakov/use-murmurhash-for-class-hashes
branch
from
d4e1f27 to
7b9b393
Compare
compiler/compiler.cmake
Outdated
| @@ -15,6 +15,7 @@ endif() | |||
|
|
|||
| prepend(KPHP_COMPILER_COMMON ${COMMON_DIR}/ | |||
| dl-utils-lite.cpp | |||
| algorithms/murmur-hash.cpp | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.