search

notify-rs / notify

🔭 Cross-platform filesystem notification library for Rust.
https://docs.rs/notify
2.67k stars 213 forks source link

Feature to opt into low-res (Windows) file ids #631

Open mqudsi opened 1 month ago

mqudsi commented 1 month ago

Thanks for the file-id crate, it looks like a great abstraction over the Windows/Unix split to help identify files by their (volume_id, file_id) tuple via a single api.

Just to recap what you already know: While NTFS uses 128-bit GUIDs to uniquely identify files on a volume, the 64-bit low-res file index in the win32 BY_HANDLE_FILE_INFORMATION struct is guaranteed to be unique (per-volume, at any rate) for all "traditional" filesystems (NTFS, FAT32, etc). 128-bit ids are only needed if you are dealing with ReFS volumes (and I hope that Microsoft had the foresight to use the lower 64-bits first before relying on the higher 64-bits to eliminate collisions, but tbh I have no clue if that's the case).

The problem is that hard-coding the high-res file id as an enum variant increases its size from 24 bytes to 32 bytes (an increase of 33%), which is fine if you are randomly storing the ids of various files to check if their inode has changed but kind of a problem for other purposes such as processing an entire "drive" composed of multiple mount points and need to store the id of all files seen. (If you're using a less-naive method of storing ids in a tree-like fashion first traversing the volume id then indexing the file id, the increase would be 50%, but the crate api only provides an opaque id, so that's besides the point).

Especially for apps primarily targeting *nix but also for apps being used under Windows and storing the id of all enumerated files, it would be nice to be able to opt out of the HighRes id altogether with a crate feature (e.g. "lowres") that would eliminate that enum member (possibly with or without removing the corresponding high-res apis) to allow a greater storage density of file ids.

Is this a change you would be open to in a PR?

dfaust commented 4 days ago

I added the high-res variant in order to be on the safe side. So I don't see a problem with tying it to a feature, as long as we add good documentation.

The question is, how do we design such a feature. I would prefer to have high-res ids enabled by default. But I don't think it's possible to remove features from transient dependencies. Alternatively we could either disable it by default or add a negative feature - which feels wrong.