[CAS] Add OnDiskTrieRawHashMap #114100
Merged
[CAS] Add OnDiskTrieRawHashMap #114100
Conversation
This file contains hidden or 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
This was referenced Oct 29, 2024
cachemeifyoucan
changed the title
cachemeifyoucan
changed the base branch from
users/cachemeifyoucan/spr/main.cas-add-ondiskhashmappedtrie
to
main
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Add OnDiskTrieRawHashMap. This is a on-disk persistent hash map that uses a Trie data structure that is similar to TrieRawHashMap. OnDiskTrieRawHashMap is thread safe and process safe. It is mostly lock free, except it internally coordinates cross process creation and closing using file lock. OnDiskTrieRawHashMap is used as the foundation to implement OnDisk CAS storage which maps hash to store data. Reviewers: Pull Request: #114100
cachemeifyoucan
force-pushed
the
users/cachemeifyoucan/spr/cas-add-ondiskhashmappedtrie
branch
from
3d10915 to
01195e5
Compare
cachemeifyoucan
assigned
bogner,
adrian-prantl and
ilovepi and unassigned
bogner,
adrian-prantl and
ilovepi
Created using spr 1.3.6
| : Hash(Hash), Data(Data.begin(), Data.size()) {} | ||
|
|
||
| ArrayRef<uint8_t> Hash; | ||
| ArrayRef<char> Data; |
There was a problem hiding this comment.
Out of curiosity — why is this using a signed char as storage?
There was a problem hiding this comment.
What suggestions you have? It is quite natural as Data can be get as StringRef which is const char*, but I guess you are asking why Hash and Data are different types? I don't really have preference either way.
| printHexDigits(OS, Bytes, LastBinaryBit, HashEndBit - LastBinaryBit); | ||
| } | ||
|
|
||
| static void appendIndexBits(std::string &Prefix, size_t Index, |
There was a problem hiding this comment.
maybe use a raw_string_stream here?
There was a problem hiding this comment.
This is mostly just bit manipulation with adding single char to string. I don't see using a stream is making it easier to read.
Created using spr 1.3.6
|
Ping |
Created using spr 1.3.7
ilovepi
reviewed
Sep 25, 2025
There was a problem hiding this comment.
I think this mostly looks fine. How much coverage are you getting out of the tests? It seems like it would be relatively low, given the implementation size. i'm not suggesting you need 100% coverage or anything like that, it just seems like there are very few tests for the amount of APIs in the header.
Also unclear on the OnDiskDataAllocator, and how it relates to the rest of this patch.
Let me make sure it covers most of the APIs. In downstream code, this is treated as an implementation details for the CAS, so I feel it is a bit wasteful to do extensive testing (like thread-safe/multi-process tests) on all layers. But I agree it should cover most of the APIs.
Going to split that up into a separate PR.
Are you suggesting just not building that when the feature is not enabled and trying to use the feature will result in link failure, instead of some error during runtime? I am ok to switch to that too. |
Created using spr 1.3.7
Created using spr 1.3.7
ilovepi
approved these changes
Sep 26, 2025
Created using spr 1.3.7
cachemeifyoucan
deleted the
users/cachemeifyoucan/spr/cas-add-ondiskhashmappedtrie
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/22097 Here is the relevant piece of the build log for the reference |
Fix: #161268 |
cachemeifyoucan
added a commit
that referenced
this pull request
Sep 29, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/21573 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/9948 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/204/builds/23593 Here is the relevant piece of the build log for the reference |
|
Fix for |
cachemeifyoucan
added a commit
that referenced
this pull request
Sep 29, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/39/builds/8066 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/13031 Here is the relevant piece of the build log for the reference |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
Add OnDiskTrieRawHashMap. This is a on-disk persistent hash map that uses a Trie data structure that is similar to TrieRawHashMap. OnDiskTrieRawHashMap is thread safe and process safe. It is mostly lock free, except it internally coordinates cross process creation and closing using file lock.
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
…lvm#161283) Link LLVM_PTHREAD_LIB from LLVMCAS library to fix rhel bots.
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.
Add OnDiskTrieRawHashMap. This is a on-disk persistent hash map that
uses a Trie data structure that is similar to TrieRawHashMap.
OnDiskTrieRawHashMap is thread safe and process safe. It is mostly lock
free, except it internally coordinates cross process creation and
closing using file lock.