On 2015-10-04 16:34, Goffredo Baroncelli wrote: > On 2015-10-04 05:37, Duncan wrote: >> Goffredo Baroncelli posted on Sat, 03 Oct 2015 19:41:33 +0200 as >> excerpted: >> >>> On 2015-10-03 12:09, Axel Burri wrote: >>>> >>>> >>>> On 2015-10-03 11:56, Goffredo Baroncelli wrote: >>>>> On 2015-10-02 18:41, a...@tty0.ch wrote: >>>>>> Old implementation used tabs "\t", and tried to work around problems >>>>>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with >>>>>> buggy output as soon as empty uuids are printed). This will never >>>>>> work correctly, as tab width is a user-defined setting in the >>>>>> terminal. >>>>> >>>>> >>>>> Why not use string_table() and table_*() functions ? >>>> >>>> string_table(), as well as all table functions by nature, needs to know >>>> the maximum size of all cells in a row before printing, and therefore >>>> buffers all the output before printing. It would eat up a lot of memory >>>> for large tables (it is not unusual to have 1000+ subvolumes in btrfs >>>> if you make heavy use of snapshotting). Furthermore, it would slow down >>>> things by not printing the output linewise. >>> >>> >>> Assuming 200bytes per row (== subvolume) x 1000 subvolumes = 200kB... I >>> don't think that this could be a problem, nor in terms of memory used >>> nor in terms of speed: if you have 1000+ subvolumes the most time >>> consuming activity is traversing the filesystem looking at the >>> snapshot... >> >> Perhaps unfortunately, scaling to millions of snapshots/subvolumes really >> *is* expected by some people. You'd be surprised at the number of folks >> that setup automated per-minute snapshotting with no automated thinning, >> and expect to be able to keep several years' worth of snapshots, and then >> wonder why btrfs maintenance commands such as balance take weeks/months... > [...] >> Obviously btrfs doesn't scale to that level now, and if it did, someone >> making the mistake of trying to get a listing of millions of snapshots >> would very likely change their mind before even hitting 10%... >> >> But that's why actually processing line-by-line is important, so they'll >> actually /see/ what happened and ctrl-C it, instead of the program >> aborting as it runs into (for example) the 32-bit user/kernel memory >> barrier, without printing anything useful... > > Please Ducan, read the code. > > *TODAY* "btrfs list" does a scan of all subvolumes storing information in > memory ! > > This is the most time consuming activity (think traversing a filesystem with > millions of snapshots) > > *Then* "btrfs list" print the info. > > So you are already blocked at the screen until all subvolume are read. And I > repeat (re)processing the information requires less time than reading the > information from the disk. > > [....] >
A quick look at the code shows me that Goffredo is right here, as __list_subvol_search() always fetches ALL data from BTRFS_IOC_TREE_SEARCH, putting it into a rbtree for later processing (assemble full paths, sorting). While there is certainly room for improvements here (assuming that BTRFS_IOC_TREE_SEARCH returns objectid's in sorted order, it would definitively be possible to produce line-by-line output), the code looks pretty elegant the way it is. I still don't think it is wise to bloat things further just for printing nice tables. My impression is that "btrfs subvolume list" is human-readable enough without the '-t' flag, while the output with '-t' flag is much more machine-readable-friendly, and thus should have the highest possible performance. e.g.: btrfs sub list -t / | (read th; while read $th ; do echo $gen; done) btrfs sub list -t | column -t Again, this is just my opinion, being a "unix-purist". Maybe a good compromise would be to use a single "\t" instead of " " as column delimiter. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
