-
Notifications
You must be signed in to change notification settings - Fork 344
[llvm][cas] Fan out persistent storage to reduce filesystem load #11101
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
base: next
Are you sure you want to change the base?
[llvm][cas] Fan out persistent storage to reduce filesystem load #11101
Conversation
The CAS OnDiskGraphDB backend currently uses a single directory for persistent storage. Over time this can lead to a very large number of files accumulating in one directory. While the point at which this overhead becomes significant on any given file system, all of them eventually reach a point where the number of entries in a single directory starts to seriously degrade the performance of an wide array of operations performed on the entries within that directory or the directory itself. In this PR we introduce an intermediate layer of subdirectories in order to spread the individual persistent files over a large number of different directories. This is the same general approach that has been taken by a wide array of other applications for exactly the same reasons. The PR currently uses 2 radix-36 characters giving an approximately 10 bit fan out. There does not seem to be a consistent bias in favour of wider or deeper fan outs, but for now a relatively wide and shallow approach seems reasonable. We current choose not to use a fan out for temporary files mostly for practical reasons. When fanning out across subdirs we need to ensure that the subdirectories exist, LLVM's current APIs for constructing temporary files do not provide a mechanism for automatically building the required directories. While we could add such functionality, that may actually hinder performance rather than helping: temporary files created by llvm are just that, and so are cleaned up at the end of execution. As a result the risk of large numbers of files accumulating is relatively low. At the same time the additional file system work needed to check for and then create new directory entries is relatively high.
I'm not super happy with the optional TempDir out parameter I use but everything else seems worse. It also seems like the CAS version should change but I don't know what the rules around that are. |
|
||
// This is around 10 bits of entropy given we're using radix-36 | ||
static const unsigned IntermediateDirNameLength = 2; | ||
static const char Radix36Chars[] = "0123456789abcdefghijklmnopqrstuvwxyz"; |
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.
@benlangmuir This is a relatively common encoding for people to use in this kind of context but I couldn't find an existing impl anywhere in the tree, and I was not sure that it was so incredibly such that it warranted adding it.
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 how I've usually seen this done in llvm:
toString(llvm::APInt(64, Hash), 36, /*Signed=*/false);
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.
Oops sent before finishing this comment:
There is also std::to_chars
, which doesn't need to allocate a string.
PersistentPath.assign(RootPath.begin(), RootPath.end()); | ||
if (TempPath) | ||
TempPath->assign(RootPath.begin(), RootPath.end()); | ||
SmallVector<char, 256> FileNameBuffer; |
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 buffer is a bit irksome, but the lack of a Twine iterator requires the copy in order to hash the filename.
We could just hash the Offset which would save the copy but given we have prefix and suffix available it seemed reasonable to use them.
I may look into the stable hash interfaces over the weekend to see if this can be performed in a better way.
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.
We should just hash the offset. The prefix is the same for every file in the CAS and each offset only corresponds to a single file, so suffix is redundant with the offset for hashing purposes.
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 requires a CAS format version bump, which you can accomplish by updating FilePrefix
at the top of OnDiskGraphDB.cpp
. It will probably require some test changes to match.
|
||
// This is around 10 bits of entropy given we're using radix-36 | ||
static const unsigned IntermediateDirNameLength = 2; | ||
static const char Radix36Chars[] = "0123456789abcdefghijklmnopqrstuvwxyz"; |
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 how I've usually seen this done in llvm:
toString(llvm::APInt(64, Hash), 36, /*Signed=*/false);
StringRef FileName = | ||
(FilePrefix + Twine(I.Offset.get()) + Suffix).toStringRef(FileNameBuffer); | ||
|
||
unsigned FileNameHash = stable_hash_name(FileName); |
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.
Unfortunately, stable_hash_...
is not stable enough for our purposes. It is documented to be stable across environments and program executions, but critically it is allowed to change in future versions, which would require a CAS format version bump.
@@ -596,7 +597,17 @@ Error OnDiskGraphDB::TempFile::keep(const Twine &Name) { | |||
assert(!Done); | |||
Done = true; | |||
// Always try to close and rename. | |||
std::error_code RenameEC = sys::fs::rename(TmpName, Name); | |||
std::error_code RenameEC = sys::fs::create_directories( | |||
sys::path::parent_path(Name.str()) |
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 wonder if we should attempt the rename first and only create the directory on failure. That will increase the cost the first time we put a file in that subdirectory (+1 rename call), but decrease it for any subsequent files (-1 mkdir). A couple of small-ish CAS databases I have sitting around have 3000+ standalone files, which would be more than 2 per sub-directory on average. It's not a big deal either way since we can change this without a format change in the future.
PersistentPath.assign(RootPath.begin(), RootPath.end()); | ||
if (TempPath) | ||
TempPath->assign(RootPath.begin(), RootPath.end()); | ||
SmallVector<char, 256> FileNameBuffer; |
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.
We should just hash the offset. The prefix is the same for every file in the CAS and each offset only corresponds to a single file, so suffix is redundant with the offset for hashing purposes.
The CAS OnDiskGraphDB backend currently uses a single directory for persistent storage. Over time this can lead to a very large number of files accumulating in one directory. While the point at which this overhead becomes significant on any given file system, all of them eventually reach a point where the number of entries in a single directory starts to seriously degrade the performance of an wide array of operations performed on the entries within that directory or the directory itself.
In this PR we introduce an intermediate layer of subdirectories in order to spread the individual persistent files over a large number of different directories. This is the same general approach that has been taken by a wide array of other applications for exactly the same reasons.
The PR currently uses 2 radix-36 characters giving an approximately 10 bit fan out. There does not seem to be a consistent bias in favour of wider or deeper fan outs, but for now a relatively wide and shallow approach seems reasonable.
We current choose not to use a fan out for temporary files mostly for practical reasons. When fanning out across subdirs we need to ensure that the subdirectories exist, LLVM's current APIs for constructing temporary files do not provide a mechanism for automatically building the required directories. While we could add such functionality, that may actually hinder performance rather than helping: temporary files created by llvm are just that, and so are cleaned up at the end of execution. As a result the risk of large numbers of files accumulating is relatively low. At the same time the additional file system work needed to check for and then create new directory entries is relatively high.