From 3958d52b163bd003bc6c0c1948aae1e34da4505b Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Tue, 4 Oct 2022 23:24:19 -0700 Subject: [PATCH 1/3] Introduce IterableDir.iterateAssumeFirstIteration This allows for avoiding an unnecessary lseek (or equivalent) call in places where it can be known that the fd has not had its cursor modified yet. --- lib/std/fs.zig | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index b1e88d2e0109..effa07a2ac55 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -811,6 +811,17 @@ pub const IterableDir = struct { }; pub fn iterate(self: IterableDir) Iterator { + return self.iterateImpl(true); + } + + /// Like `iterate`, but will not reset the directory cursor before the first + /// iteration. This should only be used in cases where it is known that the + /// `IterableDir` has not had its cursor modified yet (e.g. it was just opened). + pub fn iterateAssumeFirstIteration(self: IterableDir) Iterator { + return self.iterateImpl(false); + } + + fn iterateImpl(self: IterableDir, first_iter_start_value: bool) Iterator { switch (builtin.os.tag) { .macos, .ios, @@ -825,20 +836,20 @@ pub const IterableDir = struct { .index = 0, .end_index = 0, .buf = undefined, - .first_iter = true, + .first_iter = first_iter_start_value, }, .linux, .haiku => return Iterator{ .dir = self.dir, .index = 0, .end_index = 0, .buf = undefined, - .first_iter = true, + .first_iter = first_iter_start_value, }, .windows => return Iterator{ .dir = self.dir, .index = 0, .end_index = 0, - .first_iter = true, + .first_iter = first_iter_start_value, .buf = undefined, .name_data = undefined, }, From 2250d02dd637bf524ef8437cbe1527f16de3e260 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Tue, 4 Oct 2022 23:30:15 -0700 Subject: [PATCH 2/3] fs.deleteTree: Reduce the number of failing deleteFile calls There are two parts to this: 1. The deleteFile call on the sub_path has been moved outside the loop, since if the first call fails with `IsDir` then it's very likely that all the subsequent calls will do the same. Instead, if the `openIterableDir` call ever hits `NotDir` after the `deleteFile` hit `IsDir`, then we assume that the tree was deleted at some point and can consider the deleteTree a success. 2. Inside the `dir_it.next()` loop, we look at entry.kind and only try doing the relevant (deleteFile/openIterableDir) operation, but always fall back to the other if we get the relevant error (NotDir/IsDir). --- lib/std/fs.zig | 171 +++++++++++++++++++++++++------------------------ 1 file changed, 87 insertions(+), 84 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index effa07a2ac55..c8a88cd90e4f 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -2046,41 +2046,38 @@ pub const Dir = struct { /// this function recursively removes its entries and then tries again. /// This operation is not atomic on most file systems. pub fn deleteTree(self: Dir, sub_path: []const u8) DeleteTreeError!void { - start_over: while (true) { - var got_access_denied = false; - - // First, try deleting the item as a file. This way we don't follow sym links. - if (self.deleteFile(sub_path)) { - return; - } else |err| switch (err) { - error.FileNotFound => return, - error.IsDir => {}, - error.AccessDenied => got_access_denied = true, + // First, try deleting the item as a file. This way we don't follow sym links. + if (self.deleteFile(sub_path)) { + return; + } else |err| switch (err) { + error.FileNotFound => return, + error.IsDir => {}, + error.AccessDenied, + error.InvalidUtf8, + error.SymLinkLoop, + error.NameTooLong, + error.SystemResources, + error.ReadOnlyFileSystem, + error.NotDir, + error.FileSystem, + error.FileBusy, + error.BadPathName, + error.Unexpected, + => |e| return e, + } - error.InvalidUtf8, - error.SymLinkLoop, - error.NameTooLong, - error.SystemResources, - error.ReadOnlyFileSystem, - error.NotDir, - error.FileSystem, - error.FileBusy, - error.BadPathName, - error.Unexpected, - => |e| return e, - } + start_over: while (true) { var iterable_dir = self.openIterableDir(sub_path, .{ .no_follow = true }) catch |err| switch (err) { error.NotDir => { - if (got_access_denied) { - return error.AccessDenied; - } - continue :start_over; + // Somehow the sub_path got changed into a file while we were trying to delete the tree. + // This implies that the dir at the sub_path was deleted at some point so we consider this + // as a successful delete and return. + return; }, error.FileNotFound => { // That's fine, we were trying to remove this directory anyway. - continue :start_over; + return; }, - error.InvalidHandle, error.AccessDenied, error.SymLinkLoop, @@ -2112,63 +2109,69 @@ pub const Dir = struct { // open it, and close the original directory. Repeat. Then start the entire operation over. scan_dir: while (true) { - var dir_it = iterable_dir.iterate(); - while (try dir_it.next()) |entry| { - if (iterable_dir.dir.deleteFile(entry.name)) { - continue; - } else |err| switch (err) { - error.FileNotFound => continue, - - // Impossible because we do not pass any path separators. - error.NotDir => unreachable, - - error.IsDir => {}, - error.AccessDenied => got_access_denied = true, - - error.InvalidUtf8, - error.SymLinkLoop, - error.NameTooLong, - error.SystemResources, - error.ReadOnlyFileSystem, - error.FileSystem, - error.FileBusy, - error.BadPathName, - error.Unexpected, - => |e| return e, - } - - const new_dir = iterable_dir.dir.openIterableDir(entry.name, .{ .no_follow = true }) catch |err| switch (err) { - error.NotDir => { - if (got_access_denied) { - return error.AccessDenied; - } + var dir_it = iterable_dir.iterateAssumeFirstIteration(); + dir_it: while (try dir_it.next()) |entry| { + var treat_as_dir = entry.kind == .Directory; + handle_entry: while (true) { + if (treat_as_dir) { + const new_dir = iterable_dir.dir.openIterableDir(entry.name, .{ .no_follow = true }) catch |err| switch (err) { + error.NotDir => { + treat_as_dir = false; + continue :handle_entry; + }, + error.FileNotFound => { + // That's fine, we were trying to remove this directory anyway. + continue :dir_it; + }, + + error.InvalidHandle, + error.AccessDenied, + error.SymLinkLoop, + error.ProcessFdQuotaExceeded, + error.NameTooLong, + error.SystemFdQuotaExceeded, + error.NoDevice, + error.SystemResources, + error.Unexpected, + error.InvalidUtf8, + error.BadPathName, + error.DeviceBusy, + => |e| return e, + }; + if (cleanup_dir_parent) |*d| d.close(); + cleanup_dir_parent = iterable_dir; + iterable_dir = new_dir; + mem.copy(u8, &dir_name_buf, entry.name); + dir_name = dir_name_buf[0..entry.name.len]; continue :scan_dir; - }, - error.FileNotFound => { - // That's fine, we were trying to remove this directory anyway. - continue :scan_dir; - }, - - error.InvalidHandle, - error.AccessDenied, - error.SymLinkLoop, - error.ProcessFdQuotaExceeded, - error.NameTooLong, - error.SystemFdQuotaExceeded, - error.NoDevice, - error.SystemResources, - error.Unexpected, - error.InvalidUtf8, - error.BadPathName, - error.DeviceBusy, - => |e| return e, - }; - if (cleanup_dir_parent) |*d| d.close(); - cleanup_dir_parent = iterable_dir; - iterable_dir = new_dir; - mem.copy(u8, &dir_name_buf, entry.name); - dir_name = dir_name_buf[0..entry.name.len]; - continue :scan_dir; + } else { + if (iterable_dir.dir.deleteFile(entry.name)) { + continue :dir_it; + } else |err| switch (err) { + error.FileNotFound => continue :dir_it, + + // Impossible because we do not pass any path separators. + error.NotDir => unreachable, + + error.IsDir => { + treat_as_dir = true; + continue :handle_entry; + }, + + error.AccessDenied, + error.InvalidUtf8, + error.SymLinkLoop, + error.NameTooLong, + error.SystemResources, + error.ReadOnlyFileSystem, + error.FileSystem, + error.FileBusy, + error.BadPathName, + error.Unexpected, + => |e| return e, + } + } + } } // Reached the end of the directory entries, which means we successfully deleted all of them. // Now to remove the directory itself. From cda8df050d767006cabd618142220d4c880529bd Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Tue, 4 Oct 2022 23:30:50 -0700 Subject: [PATCH 3/3] fs.Dir: Add deleteTreeRecursive --- lib/std/fs.zig | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index c8a88cd90e4f..3527efe9dd1e 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -2197,6 +2197,85 @@ pub const Dir = struct { } } + fn deleteTreeRecursiveImpl(dir: Dir, sub_path: []const u8, kind_hint: File.Kind) DeleteTreeError!void { + start_over: while (true) { + var iterable_dir = iterable_dir: { + var treat_as_dir = kind_hint == .Directory; + + handle_entry: while (true) { + if (treat_as_dir) { + break :iterable_dir dir.openIterableDir(sub_path, .{ .no_follow = true }) catch |err| switch (err) { + error.NotDir => { + treat_as_dir = false; + continue :handle_entry; + }, + error.FileNotFound => { + // That's fine, we were trying to remove this directory anyway. + return; + }, + + error.InvalidHandle, + error.AccessDenied, + error.SymLinkLoop, + error.ProcessFdQuotaExceeded, + error.NameTooLong, + error.SystemFdQuotaExceeded, + error.NoDevice, + error.SystemResources, + error.Unexpected, + error.InvalidUtf8, + error.BadPathName, + error.DeviceBusy, + => |e| return e, + }; + } else { + if (dir.deleteFile(sub_path)) { + return; + } else |err| switch (err) { + error.FileNotFound => return, + + error.NotDir => unreachable, + + error.IsDir => { + treat_as_dir = true; + continue :handle_entry; + }, + + error.AccessDenied, + error.InvalidUtf8, + error.SymLinkLoop, + error.NameTooLong, + error.SystemResources, + error.ReadOnlyFileSystem, + error.FileSystem, + error.FileBusy, + error.BadPathName, + error.Unexpected, + => |e| return e, + } + } + } + }; + defer iterable_dir.close(); + + var dir_it = iterable_dir.iterateAssumeFirstIteration(); + while (try dir_it.next()) |entry| { + try deleteTreeRecursiveImpl(iterable_dir.dir, entry.name, entry.kind); + } + + dir.deleteDir(sub_path) catch |err| switch (err) { + error.FileNotFound => return, + error.DirNotEmpty => continue :start_over, + else => |e| return e, + }; + return; + } + } + + pub fn deleteTreeRecursive(self: Dir, sub_path: []const u8) DeleteTreeError!void { + return deleteTreeRecursiveImpl(self, sub_path, .File); + } + /// Writes content to the file system, creating a new file if it does not exist, truncating /// if it already exists. pub fn writeFile(self: Dir, sub_path: []const u8, data: []const u8) !void {