Various performance fixes #115
Merged
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
Contributor
eryugey
commented
Apr 14, 2023
eryugey
commented
- convert generic MultikeyBTreeMap to a app-specific InodeStore
- optimize virtio transport to avoid expending VecDeque
- add config to control mnt id in InodeAltKey
- add config to specify dir-entry-timeout and dir-attr-timeout
- remove some trace logs in hot paths
In commit 095d821 ("passthrough: add mnt id as inode alt key") we added mntid as part of inode alt key, and use name_to_handle_at(2) to get mntid. And recent perf tests show that name_to_handle_at(2) is kind of expensive in lookup intensive workloads. So add a knob to control it, and default to false. Signed-off-by: Eryu Guan <[email protected]>
For AI training workloads, APP reads large number of files from dataset and only reads them once. So entry and attr cache for such files are not needed, and large number of cached entry will make InodeMap bigger and bigger, and make InodeMap re-allocate its memory over and over again, which hits performance. On the other hand, the entry cache for dir is very helpful. But entry_timeout and attr_timeout will set the same timeout value for both dirs and regular files. So introduce 'dir_entry_timeout' and 'dir_attr_timeout' config options to config dir entry/attr separately. So that we could use cache=none (which indicates entry/attr timeout as 0 and bypass guest pagecache to avoid massive memory copy) and set dir_entry_timeout and dir_attr_timeout as none-zero to cache dirs. Signed-off-by: Eryu Guan <[email protected]>
The implementation of MultiKeyBTreeMap is for generic usage and may not
be efficient in passthroughfs, e.g. it has a vector in alt key and when
InodeMap grows, it triggers many memory re-allocations, and hits
performance.
So introduce application-specific InodeStore struct to replace the
generic MultiKeyBTreeMap. This follows virtiofsd-rs commit 4d27d3ec772f
("passthrough: Use application-specific inode storage")
Signed-off-by: Eryu Guan <[email protected]>
lookup() and forget() are two hot paths, we see performance hit even if trace log is not enabled. So remove trace log in hot paths. Signed-off-by: Eryu Guan <[email protected]>
Expending VecDeque might be expensive, so pre-allocating a vecDeque with a fixed 64 capacity and hope it could hold all slices we need. And it will expand if it's not big enough anyway. Signed-off-by: Eryu Guan <[email protected]>
eryugey
requested review from
bergwolf,
imeoer,
jiangliu and
liubogithub
as code owners
Signed-off-by: Eryu Guan <[email protected]>
bergwolf
reviewed
Apr 18, 2023
| .unwrap(); | ||
| // Allocate VecDeque with 64 capacity and hope it could hold all slices to avoid expending | ||
| // VecDeque repeatedly. | ||
| let mut buffers = VecDeque::with_capacity(64); |
There was a problem hiding this comment.
I wonder if we can just stop at some point instead of grouping all the readable descriptors every time a chain is accessed.
There was a problem hiding this comment.
Let's merge as is and look for more optimization in future.
bergwolf
approved these changes
Apr 19, 2023
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.