-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This is a perf-improved gcsfuse package used internally.
- Loading branch information
Showing
4 changed files
with
182 additions
and
0 deletions.
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
# Copyright lowRISC Contributors. | ||
# Licensed under the MIT License, see LICENSE for details. | ||
# SPDX-License-Identifier: MIT | ||
{ | ||
gcsfuse, | ||
fetchFromGitHub, | ||
}: | ||
gcsfuse.override (prev: { | ||
buildGoModule = args: | ||
prev.buildGoModule (args | ||
// rec { | ||
version = "2.4.0"; | ||
|
||
src = fetchFromGitHub { | ||
owner = "googlecloudplatform"; | ||
repo = "gcsfuse"; | ||
rev = "v${version}"; | ||
hash = "sha256-4susiXFe1aBcakxRkhmOe7dvcwsNfam4KKyFFzYXhcU="; | ||
}; | ||
|
||
vendorHash = "sha256-uOr929RS8q7LB+WDiyxEIyScE/brmvPJKfnq28PfsDM="; | ||
|
||
patches = [ | ||
# https://github.com/GoogleCloudPlatform/gcsfuse/pull/2269 | ||
./dentry-cache.patch | ||
# https://github.com/GoogleCloudPlatform/gcsfuse/pull/2285 | ||
./symlink-cache.patch | ||
]; | ||
}); | ||
}) |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
From e1575385af3f980b88e3197bf524dc4c6ee15148 Mon Sep 17 00:00:00 2001 | ||
From: Gary Guo <[email protected]> | ||
Date: Fri, 2 Aug 2024 11:32:48 +0100 | ||
Subject: [PATCH 1/2] Set FUSE entry expiration time | ||
|
||
Currently entry expiration is not set, which means that the dentry is | ||
thrown away after each use, causing significant overhead since every | ||
lookup operation needs to call into userspace. | ||
|
||
Entry captures the mapping from file name to inode. The kernel list | ||
cache option allows readdir response to be cached, which is also | ||
dentry, so I reuse the same TTL for entry expiration timeout. | ||
This is conservative, and I am pretty sure there can be further | ||
performance improvement by setting the expiration in more cases (as the | ||
object generation number is also cached). | ||
|
||
There are a few other places where dentry is returned, e.g. create or | ||
mkdir, but since the cache is more useful for readonly/read-mostly FS, I | ||
didn't bother setting the dentry expiration time for these calls. | ||
|
||
This change brings significant performance improvement due for readonly | ||
workloads. | ||
* I tested the change with a bucket that have 6000 files, using `du -hs` | ||
on the directory (it will stat all the files.). I ran the command once | ||
to populate the cache, and then time the second run. The change | ||
reduces time taken by the command from 532ms to 72ms. | ||
* I also tested the change in a production compilation workload. The | ||
time taken for each run reduces from 7 minutes to 1 minute. | ||
|
||
Signed-off-by: Gary Guo <[email protected]> | ||
--- | ||
internal/fs/fs.go | 6 ++++++ | ||
1 file changed, 6 insertions(+) | ||
|
||
diff --git a/internal/fs/fs.go b/internal/fs/fs.go | ||
index ba3e9f57cb..47de50323c 100644 | ||
--- a/internal/fs/fs.go | ||
+++ b/internal/fs/fs.go | ||
@@ -1358,6 +1358,12 @@ func (fs *fileSystem) LookUpInode( | ||
return err | ||
} | ||
|
||
+ // If list cache is enabled, directory entries returned by ReadDir may be cached. | ||
+ // So we can also cache the directory entries returned by Lookup. | ||
+ if fs.kernelListCacheTTL > 0 { | ||
+ e.EntryExpiration = time.Now().Add(fs.kernelListCacheTTL) | ||
+ } | ||
+ | ||
return | ||
} | ||
|
||
|
||
From 85a9480bea86829af64cb6e7da31a0ae4cdceee7 Mon Sep 17 00:00:00 2001 | ||
From: Gary Guo <[email protected]> | ||
Date: Mon, 5 Aug 2024 13:50:10 +0100 | ||
Subject: [PATCH 2/2] Enable negative dentry caching | ||
|
||
This can bring perf improvement in compilation workload where the | ||
compiler searches for headers. | ||
|
||
Signed-off-by: Gary Guo <[email protected]> | ||
--- | ||
internal/fs/fs.go | 10 ++++++++++ | ||
1 file changed, 10 insertions(+) | ||
|
||
diff --git a/internal/fs/fs.go b/internal/fs/fs.go | ||
index 47de50323c..f3c397db5b 100644 | ||
--- a/internal/fs/fs.go | ||
+++ b/internal/fs/fs.go | ||
@@ -1344,6 +1344,16 @@ func (fs *fileSystem) LookUpInode( | ||
// Find or create the child inode. | ||
child, err := fs.lookUpOrCreateChildInode(ctx, parent, op.Name) | ||
if err != nil { | ||
+ // If both list cache and nonexistent type cache is enabled, we also | ||
+ // instruct FUSE to cache negative entries. | ||
+ if err == fuse.ENOENT && fs.enableNonexistentTypeCache && fs.kernelListCacheTTL > 0 { | ||
+ // Inode 0 is equal to ENOENT return, but allows FUSE to cache the response. | ||
+ err = nil | ||
+ e := &op.Entry | ||
+ e.Child = 0 | ||
+ e.EntryExpiration = time.Now().Add(fs.kernelListCacheTTL) | ||
+ return | ||
+ } | ||
return err | ||
} | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
From 22a47caa4826d68bb5d2b70c31261b50fb441a55 Mon Sep 17 00:00:00 2001 | ||
From: Gary Guo <[email protected]> | ||
Date: Mon, 5 Aug 2024 12:44:11 +0100 | ||
Subject: [PATCH 1/2] Fix size of symlinks | ||
|
||
Symlink inodes are expected to have size equal to the length of its | ||
target. This is defined in POSIX spec. | ||
|
||
FUSE's symlink cache also expects this, so fixing this prepares the | ||
enabling of the symlink cache. | ||
|
||
Signed-off-by: Gary Guo <[email protected]> | ||
--- | ||
internal/fs/inode/symlink.go | 4 ++++ | ||
1 file changed, 4 insertions(+) | ||
|
||
diff --git a/internal/fs/inode/symlink.go b/internal/fs/inode/symlink.go | ||
index 59db3079b7..145e431718 100644 | ||
--- a/internal/fs/inode/symlink.go | ||
+++ b/internal/fs/inode/symlink.go | ||
@@ -84,6 +84,10 @@ func NewSymlinkInode( | ||
Atime: m.Updated, | ||
Ctime: m.Updated, | ||
Mtime: m.Updated, | ||
+ // POSIX spec requires that for symbolic links, size attribute | ||
+ // reflects the length in bytes of the pathname contained in the | ||
+ // symbolic link. This is also expected by symlink cache in FUSE. | ||
+ Size: uint64(len(m.Metadata[SymlinkMetadataKey])), | ||
}, | ||
target: m.Metadata[SymlinkMetadataKey], | ||
} | ||
|
||
From 8ef9d3d056f7b2c31469d6a119cfead366bda35c Mon Sep 17 00:00:00 2001 | ||
From: Gary Guo <[email protected]> | ||
Date: Sat, 3 Aug 2024 21:44:47 +0100 | ||
Subject: [PATCH 2/2] Enable kernel symlink caching | ||
|
||
This would provide significant savings when symlink'ed path is | ||
frequently accessed. | ||
|
||
Since symlink target is not mutable, this is always safe regardless TTL | ||
settings. (When the symlink is removed and re-created, its generatiton | ||
would change so a different inode is created). | ||
|
||
Signed-off-by: Gary Guo <[email protected]> | ||
--- | ||
cmd/mount.go | 3 +++ | ||
1 file changed, 3 insertions(+) | ||
|
||
diff --git a/cmd/mount.go b/cmd/mount.go | ||
index c8f19841c0..1fec7b9599 100644 | ||
--- a/cmd/mount.go | ||
+++ b/cmd/mount.go | ||
@@ -174,6 +174,9 @@ func getFuseMountConfig(fsName string, newConfig *cfg.Config, mountConfig *confi | ||
// access two files under same directory parallely, then the lookups also | ||
// happen parallely. | ||
EnableParallelDirOps: !(mountConfig.FileSystemConfig.DisableParallelDirops), | ||
+ // Symlink target is not mutable (removing and re-creating would cause a different | ||
+ // inode to be created), so it's safe to enable symlink caching. | ||
+ EnableSymlinkCaching: true, | ||
} | ||
|
||
mountCfg.ErrorLogger = logger.NewLegacyLogger(logger.LevelError, "fuse: ") |