Skip to content
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

feat: show Btrfs subvolumes #1176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

horror-proton
Copy link

@horror-proton horror-proton commented Oct 4, 2024

Show Btrfs subvolumes as underlined blue text.
A similar version was implemented in #53 , but not been merged as in #167 .

Some questions:

I came up with two ways to tell if a file is in Btrfs,

1. Searching for parent directory recursively in /proc/mounts
  • Mounted filesystems are already listed, no extra operation required.
  • May not be fully reliable (e.g. in some filesystem namespace such as a container or bwrap, procfs may not even exist)
    [@ArchLinux] > bwrap --symlink /usr/lib64 /lib64 --ro-bind /usr /usr findmnt
    findmnt: can't read /proc/mounts: No such file or directory
    
2. Using glibc's statfs and check f_type
  • Requires an extra statfs syscall
  • Kernel also reads out many other information, such as capacity. Could affect performance?
  • Benefit from cache mechanism of pathname lookup in kernel? I don't know.

I'm not sure which one is better and how to benchmark.

Here I just kept them both and the second one only works as a fallback.

I have no idea how all the theming works, I just copy the codes from mount_point.

Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure which one is better and how to benchmark.

you can run a version with and without changes on a couple of different directories with a bunch of flags using hyperfine and see if it indicates stuff getting slower

src/fs/file.rs Outdated Show resolved Hide resolved
@daviessm
Copy link
Contributor

daviessm commented Oct 6, 2024

I'm the contributor of #53 and #167, for the record I'm glad someone did this - I just haven't had time (read: I'm too lazy). Hope to see it merged!

MartinFillon
MartinFillon previously approved these changes Oct 6, 2024
Copy link
Contributor

@MartinFillon MartinFillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

src/fs/file.rs Outdated Show resolved Hide resolved
@horror-proton
Copy link
Author

I ran some simple tests on a loop device formatted into Btrfs,

> ls -lah
drwxr-xr-x ... 0
lrwxrwxrwx ... dir -> 0/0/0/0/0/0/0/0/0/0
> cd dir
> seq 1000 | xargs btrfs subvolume create

Searching for parent mountpoint recursively:

Benchmark 1: eza -lah --mounts .
  Time (mean ± σ):      22.6 ms ±   1.5 ms    [User: 35.4 ms, System: 10.0 ms]
  Range (min … max):    19.8 ms …  30.7 ms    118 runs

statfs syscall:

Benchmark 1: eza -lah --mounts .
  Time (mean ± σ):      16.9 ms ±   1.4 ms    [User: 32.3 ms, System: 10.9 ms]
  Range (min … max):    14.2 ms …  22.3 ms    170 runs

Do nothing and return false:

Benchmark 1: /tmp/eza/target/debug/eza -lah --mounts .
  Time (mean ± σ):      16.7 ms ±   1.4 ms    [User: 33.0 ms, System: 11.3 ms]
  Range (min … max):    13.5 ms …  20.5 ms    185 runs

@daviessm
Copy link
Contributor

daviessm commented Oct 7, 2024

You'll probably need to test with a few (hundred) thousand more, as well as nested directories, nested subvolumes, and if possible slow (HDD) devices for each of the options.

@horror-proton
Copy link
Author

My laptop has no HDD attached to it, so I'm using dmsetup to simulate a device with 20ms of delay based on the same loop device.
The first ls attempt took 16 seconds !!!, however the following benchmarks had almost the same result as the ones on a normal loop device (24ms and 17ms).
Presumably the kernel has caching mechanism for files and filesystem info. I'm not sure if the result also applies to a real word situation, for the test filesystem is significantly smaller than my memory.

src/fs/file.rs Outdated Show resolved Hide resolved
@horror-proton

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

5 participants