-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[CAS] Add UnifiedOnDiskCache and OnDiskCAS #114103
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
[CAS] Add UnifiedOnDiskCache and OnDiskCAS #114103
Conversation
Add OnDiskCAS abstraction, that implements ObjectStore and ActionCache interface using OnDiskGraphDB and OnDiskKeyValueDB. Reviewers: Pull Request: #114103
94172a6 to
4b951ef
Compare
Created using spr 1.3.7
Created using spr 1.3.7
| Error validate() const final; | ||
|
|
||
| private: | ||
| std::shared_ptr<ondisk::UnifiedOnDiskCache> UniDB; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm missing it, but why does this need to be a shared_ptr? I don't see obvious uses where we're really relying on the refcount here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just extending the lifetime of the wrapped UnifiedOnDiskCache to make sure it is not deleted when ActionCache is still in use.
Alternatively, we can leave this as a raw pointer and left the life-time management to user, but since this is not really a ref-count that is being updated often, this safe guard is pretty safe to keep.
Created using spr 1.3.7
adrian-prantl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate how well-documented the interface is!
Created using spr 1.3.7
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/14/builds/4601 Here is the relevant piece of the build log for the reference |
|
I saw two new test failures with this patch when I build with EXPENSIVE_CHECKS. And with later compiler/header versions I saw it explicitly pointing out UnifiedOnDiskCache.cpp:176: |
|
That is a bug in the compare function. Fix here: #166963 |
|
Looks like this commit introduced a flaky test. |
Sorry I missed that. Do you know which windows version is used on that bot? The description only says windows 10, but I am very interested to know if that windows version will trigger this condition: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Windows/Path.inc#L966 |
|
@gkistanova Let me disable the test for now on windows to see if version is the problem: #170063 |
|
That computer is running version 22H2 (OS Build 19045.5854). |
This is newer than my suspected version. Let me disable more versions as workaround. |
| #include <optional> | ||
|
|
||
| #if __has_include(<sys/sysctl.h>) | ||
| #include <sys/sysctl.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing a failure due to this with my build. The sys/sysctl.h header contains the following warning:
#warning "The <sys/sysctl.h> header is deprecated and will be removed
As a result, the compiler throws:
In file included from /localhome/local-adeodhar/llvm-solid-2/llvm/lib/CAS/UnifiedOnDiskCache.cpp:92:
/usr/include/x86_64-linux-gnu/sys/sysctl.h:21:2: error: #warning "The <sys/sysctl.h> header is deprecated and will be removed." [-Werror=cpp]
21 | #warning "The <sys/sysctl.h> header is deprecated and will be removed."
| ^~~~~~~
cc1plus: all warnings being treated as errors
The __has_include does not prevent the compiler from throwing an error when warnings-as-errors is enabled. I understand that this does not cause a problem for the default LLVM build (which does not have warnings-as-errors on), but might cause pain to users who have Werror enabled on a system that has sys/sysctl.h deprecated.
I might be missing something- can you help me figure out how to work around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which distro are you using? Maybe the better default is to only use this on Darwin. I will do some testing to see what is the best for code portability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using Ubuntu 20.04.
Distributor ID: Ubuntu
Description: Ubuntu 20.04.6 LTS
Release: 20.04
Codename: focal
Add a new abstraction layer UnifiedOnDiskCache that adds new functions
of disk space management and data validation that builds on top of
OnDiskGraphDB and OnDiskKeyValueDB.
Build upon UnifiedOnDiskCache, it is OnDiskCAS that implements
ObjectStore and ActionCache interface for LLVM tools to interact with
CAS storage.