Add entry counts for directories #965

zamu-flowerpot · 2022-11-09T22:41:26Z

This would resolve issue #306 with entry counts for directories.
This also resolves issue #964 as I had to define how to compare EntrySize::EntryCount and EntrySize::Bytes.

This adds a single dependency walkdir to handle counting the number of items in a directory cross-platform. Also, by default it doesn't follow symbolic links which seems fine.

`src/listing.rs` === * Add `EntrySize` struct * Impl Display for EntrySize * Change `Option<bytesize::ByteSize>` to `EntrySize` in `Entry` * Add Entry::Directory size calculation utilizing WalkDir * Sort directory listings by always putting directories at the top and sorting files and directories based on their own semantics (ie. entry counts for directories and bytes for files) `src/renderer.rs` === * Change `entry_raw` from using `if-let` to just operating on `entry.size`

zamu-flowerpot · 2022-11-09T22:53:58Z

One thing I didn't look into was setting these behind CLI flags and this probably needs some memoization as it can be just slightly noticeable when looking at parents with a lot of children (or maybe just limiting it down to direct children).

svenstaro · 2022-11-10T12:12:29Z

I think this should probably be behind CLI flags as the performance hit can be severe and we don't want that by default. I will do a proper review a little later.

svenstaro · 2022-11-10T12:16:15Z

Cool stuff! As you already said, a memoization technique would be neat. I think a HashMap would be good enough for us. What do you also think about timing out on directories that just take too long? I don't miniserve to ever not feel snappy when generating pages.

svenstaro · 2022-11-10T12:13:05Z

src/renderer.rs

+                            // @if let size = entry.size {
+                            //     span.mobile-info.size {
+                            //         (maud::display(size))
+                            //     }
+                            // }
                        }
                    }
                }
            }
            td.size-cell {
-                @if let Some(size) = entry.size {
-                    (maud::display(size))
-                }
+                (maud::display(&entry.size))
+                // @if let size = entry.size {
+                //     (maud::display(size))
+                // }


What's with the commented code here?

Ah, I didn't remove that since it was the code that would need to be switched on if we were going to throw everything behind a feature flag.

svenstaro · 2022-11-10T12:13:18Z

src/listing.rs

+            // e2.size
+            //     .unwrap_or_else(|| ByteSize::b(0))
+            //     .cmp(&e1.size.unwrap_or_else(|| ByteSize::b(0)))


svenstaro · 2022-11-10T12:14:25Z

src/listing.rs

+#[derive(Clone, Copy, PartialEq, Eq)]
+/// Possible entry size types
+pub enum EntrySize {
+    /// EntryCount is number of entries in a directory
+    EntryCount(usize),
+    /// Bytes is number of bytes in a file
+    Bytes(bytesize::ByteSize),
+}
+
+impl Display for EntrySize {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            EntrySize::EntryCount(count) => write!(f, "{}", count),
+            EntrySize::Bytes(bytes) => write!(f, "{}",  bytes),
+        }
+    }
+}
+


Wouldn't it be cool to also show the combined directory file size?

Not sure what you are referring to by "combined directory file size". Are you referring to the total size in bytes of a directory? That might be a bit expensive and probably would require a persistent cache to be anywhere near fast.

Yes, the byte sizes. I recognize it might be too slow. Hm. If you like to, you could try to experiment with this.

zamu-flowerpot · 2022-11-10T13:39:10Z

Cool stuff! As you already said, a memoization technique would be neat. I think a HashMap would be good enough for us. [...]

A HashMap would be fine. I tried my hand at using a once_cell::OnceCell<HashMap>, but couldn't figure out how to make a mutable global static. Maybe it'd just be as simple as just constructing it and passing it in as part of the function call to directory_listing?

[...] What do you also think about timing out on directories that just take too long? I don't miniserve to ever not feel snappy when generating pages.

Definitely. We might be able to shortcut a lot of the issues by just counting direct children by changing the size calculation to call .max_depth(1). It still might be slow, but it's on the user if they have a directory with a million files right?

zamu-flowerpot · 2022-11-10T13:42:04Z

src/listing.rs

@@ -256,12 +276,14 @@ pub fn directory_listing(
                    Err(_) => None,
                };

+                let size = WalkDir::new(entry.path()).into_iter().count();


Thinking about it, we might be able to just do .take(UPPER_LIMIT).count() instead of .count() to shortcut execution time. So, we might have UPPER_LIMIT be something like 10001 and if it's more than 10000 we have the Impl Display for EntrySize print >10000 or something along those lines.

The other option would have been to add an actual timeout, but I'm not really sure how to best do that in a non-async context.

svenstaro · 2022-11-11T03:14:00Z

Cool stuff! As you already said, a memoization technique would be neat. I think a HashMap would be good enough for us. [...]

A HashMap would be fine. I tried my hand at using a once_cell::OnceCell<HashMap>, but couldn't figure out how to make a mutable global static. Maybe it'd just be as simple as just constructing it and passing it in as part of the function call to directory_listing?

You're going to need a OnceCell<Arc<Mutex<HashMap>>>.

[...] What do you also think about timing out on directories that just take too long? I don't miniserve to ever not feel snappy when generating pages.

Definitely. We might be able to shortcut a lot of the issues by just counting direct children by changing the size calculation to call .max_depth(1). It still might be slow, but it's on the user if they have a directory with a million files right?

How about walking through the tree in threads using rayon? Should be fairly easy and it might also allow you to cancel/timeout threads.

`Cargo.lock` & `Cargo.toml` === * Remove `WalkDir` * Add `fs_extra` * Add `OnceCell` `src/listing.rs` === * remove `use walkdir::WalkDir` * add HashMap, Arc, Mutex, OnceCell, log::warn, and `FILE_SIZE_CACHE` * Cargo fmt

`src/listing.rs` === * Cargo fmt * Replace `WalkDir::new(entry.path()).into_iter().count()` with calculating directory size using `fs_extra::dir::get_size` * If `get_size` fails, fall back to `std::fs::read_dir` counting

zamu-flowerpot · 2022-11-21T01:51:31Z

You're going to need a OnceCell<Arc<Mutex<HashMap>>>.

Yep, worked like a charm. Thanks for the hint.

How about walking through the tree in threads using rayon? Should be fairly easy and it might also allow you to cancel/timeout threads.

Tried rayon and then encountered BurntSushi/walkdir#21. This lead me to look for something other than WalkDir and found fs_extra.

I just wanted to get this to a MVP and figure out what flags we want to incorporate (and how to wire them from the config/cli to the call site). So for now I left off the parallel scanning.

Looking at fs_extras code, it might be doable to parallelize the size scanning using Rayon. If we get all the decedent DirEntrys and push them onto a stack, for_each over the stack and cache their size, and then calculate the directory sizes from the cache on request, we should be pretty fast. The biggest issue still comes down to cache invalidation though it wouldn't be too hard to change the cache map to include modified time.

That said another option is to spawn a thread that listens for file system notifications and updates the cache on writes, but I'm only aware of Linux's inotify so I'm not sure if it's really cross-platform.

Let me know if you have any questions or concerns.

svenstaro · 2025-02-07T13:21:06Z

@zamu-flowerpot Hey, would you like to rebase this and clean it up? I'd like to get this feature into the next release.

zamu-flowerpot · 2025-02-10T21:37:45Z

It's been 2 years and I don't particularly have the time to tinker with this.

I did take a look at it though and fs_extra is long since abandoned, so some solution for indexing the directories is needed. All the other conflicts are minor things.

svenstaro · 2025-02-11T06:30:57Z

It's been 2 years and I don't particularly have the time to tinker with this.

I did take a look at it though and fs_extra is long since abandoned, so some solution for indexing the directories is needed. All the other conflicts are minor things.

Fair enough.

svenstaro · 2025-03-07T11:01:12Z

Closing this in favor of #1482.

0x9394 · 2025-03-19T02:53:39Z

the counting triggered on every page, even is same session.
like open page 1, click folder x, then goes back page 1, it will trigger 3 times folder size calculation.

and with multiple folders and large amount of files on HDD the counting takes very long.
(yes the page is responsive but when I try open the same page from 3 computers, the HDD became very very busy, as I writing this the HDD is busy for 5 minutes and still not finished, if more users come in the situation will be really bad)

please consider add option to disable this counting behavior (or cache it for a long time for all users) . than you. @svenstaro

svenstaro · 2025-03-19T03:18:55Z

I'm aware, that's why there's been no release of this yet. I'm not really happy with the current mechanism. The current goal is to make this cancelable and also for the server to cache it. Also there should be no concurrent calculations of the same directory.

0x9394 · 2025-03-19T03:55:24Z

with docker latest(0.29.0) . it has folder size calc triggered.

just found the container use 386.8MiB of ram. should be related to the size calc function.

0x9394 · 2025-03-19T04:05:53Z

or consider just show file size (when files a listed in page, ignore folder size/file count) ? that would be much faster.

svenstaro · 2025-03-19T18:58:37Z

Well there was a feature request to add folder counting. I'll add a flag for disabling this today for the time being.

svenstaro · 2025-03-20T12:52:39Z

Directory size counting is now disabled by default and can be enabled via --directory-size.

zamu-flowerpot added 2 commits November 9, 2022 17:29

Add 'WalkDir' dependency

c1cc958

svenstaro reviewed Nov 10, 2022

View reviewed changes

zamu-flowerpot commented Nov 10, 2022

View reviewed changes

zamu-flowerpot added 3 commits November 20, 2022 20:17

feat/issue-306-pr:

1c71e93

`Cargo.lock` & `Cargo.toml` === * Remove `WalkDir` * Add `fs_extra` * Add `OnceCell` `src/listing.rs` === * remove `use walkdir::WalkDir` * add HashMap, Arc, Mutex, OnceCell, log::warn, and `FILE_SIZE_CACHE` * Cargo fmt

feat/issue-306-pr:

dc734c4

`src/listing.rs` === * Cargo fmt * Replace `WalkDir::new(entry.path()).into_iter().count()` with calculating directory size using `fs_extra::dir::get_size` * If `get_size` fails, fall back to `std::fs::read_dir` counting

Merge branch 'master' into feat/issue-306-pr

70f8b3e

svenstaro changed the title ~~feat/issue-306-pr~~ Add entry counts for directories Feb 7, 2025

svenstaro mentioned this pull request Mar 7, 2025

Add asynchronous directory size counting #1482

Merged

svenstaro closed this Mar 7, 2025

Uh oh!

Add entry counts for directories #965

Add entry counts for directories #965

Uh oh!

Conversation

zamu-flowerpot commented Nov 9, 2022

Uh oh!

zamu-flowerpot commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenstaro commented Nov 10, 2022

Uh oh!

svenstaro commented Nov 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zamu-flowerpot commented Nov 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svenstaro commented Nov 11, 2022

Uh oh!

zamu-flowerpot commented Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenstaro commented Feb 7, 2025

Uh oh!

zamu-flowerpot commented Feb 10, 2025

Uh oh!

svenstaro commented Feb 11, 2025

Uh oh!

svenstaro commented Mar 7, 2025

Uh oh!

0x9394 commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenstaro commented Mar 19, 2025

Uh oh!

0x9394 commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x9394 commented Mar 19, 2025

Uh oh!

svenstaro commented Mar 19, 2025

Uh oh!

svenstaro commented Mar 20, 2025

Uh oh!

Uh oh!

zamu-flowerpot commented Nov 9, 2022 •

edited

Loading

zamu-flowerpot commented Nov 21, 2022 •

edited

Loading

0x9394 commented Mar 19, 2025 •

edited

Loading

0x9394 commented Mar 19, 2025 •

edited

Loading