Skip to content

Commit

Permalink
Merge pull request #14833 from QusaiHroub/optimize_std.fs.Dir.makeOpe…
Browse files Browse the repository at this point in the history 
…nPath_12474

#12474: std.fs.Dir.makeOpenPath: optimize case, if path already exists
  • Loading branch information
@andrewrk
andrewrk authored Oct 22, 2023
2 parents 7d50634 + fb5f69a commit e354aac
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 21 deletions.
103 changes: 90 additions & 13 deletions lib/std/fs.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1459,8 +1459,9 @@ pub const Dir = struct {
try os.mkdiratW(self.fd, sub_path, default_new_dir_mode);
}

/// Calls makeDir recursively to make an entire path. Returns success if the path
/// already exists and is a directory.
/// Calls makeDir iteratively to make an entire path
/// (i.e. creating any parent directories that do not exist).
/// Returns success if the path already exists and is a directory.
/// This function is not atomic, and if it returns an error, the file system may
/// have been modified regardless.
pub fn makePath(self: Dir, sub_path: []const u8) !void {
Expand All @@ -1483,22 +1484,89 @@ pub const Dir = struct {
}
}

/// Calls makeOpenDirAccessMaskW iteratively to make an entire path
/// (i.e. creating any parent directories that do not exist).
/// Opens the dir if the path already exists and is a directory.
/// This function is not atomic, and if it returns an error, the file system may
/// have been modified regardless.
fn makeOpenPathAccessMaskW(self: Dir, sub_path: []const u8, access_mask: u32, no_follow: bool) OpenError!Dir {
const w = os.windows;
var it = try path.componentIterator(sub_path);
// If there are no components in the path, then create a dummy component with the full path.
var component = it.last() orelse path.NativeUtf8ComponentIterator.Component{
.name = "",
.path = sub_path,
};

while (true) {
const sub_path_w = try w.sliceToPrefixedFileW(self.fd, component.path);
const is_last = it.peekNext() == null;
var result = self.makeOpenDirAccessMaskW(sub_path_w.span().ptr, access_mask, .{
.no_follow = no_follow,
.create_disposition = if (is_last) w.FILE_OPEN_IF else w.FILE_CREATE,
}) catch |err| switch (err) {
error.FileNotFound => |e| {
component = it.previous() orelse return e;
continue;
},
else => |e| return e,
};

component = it.next() orelse return result;
// Don't leak the intermediate file handles
result.close();
}
}

/// This function performs `makePath`, followed by `openDir`.
/// If supported by the OS, this operation is atomic. It is not atomic on
/// all operating systems.
/// On Windows, this function performs `makeOpenPathAccessMaskW`.
pub fn makeOpenPath(self: Dir, sub_path: []const u8, open_dir_options: OpenDirOptions) !Dir {
// TODO improve this implementation on Windows; we can avoid 1 call to NtClose
try self.makePath(sub_path);
return self.openDir(sub_path, open_dir_options);
return switch (builtin.os.tag) {
.windows => {
const w = os.windows;
const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA |
w.SYNCHRONIZE | w.FILE_TRAVERSE;

return self.makeOpenPathAccessMaskW(sub_path, base_flags, open_dir_options.no_follow);
},
else => {
return self.openDir(sub_path, open_dir_options) catch |err| switch (err) {
error.FileNotFound => {
try self.makePath(sub_path);
return self.openDir(sub_path, open_dir_options);
},
else => |e| return e,
};
},
};
}

/// This function performs `makePath`, followed by `openIterableDir`.
/// If supported by the OS, this operation is atomic. It is not atomic on
/// all operating systems.
pub fn makeOpenPathIterable(self: Dir, sub_path: []const u8, open_dir_options: OpenDirOptions) !IterableDir {
// TODO improve this implementation on Windows; we can avoid 1 call to NtClose
try self.makePath(sub_path);
return self.openIterableDir(sub_path, open_dir_options);
return switch (builtin.os.tag) {
.windows => {
const w = os.windows;
const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA |
w.SYNCHRONIZE | w.FILE_TRAVERSE | w.FILE_LIST_DIRECTORY;

return IterableDir{
.dir = try self.makeOpenPathAccessMaskW(sub_path, base_flags, open_dir_options.no_follow),
};
},
else => {
return self.openIterableDir(sub_path, open_dir_options) catch |err| switch (err) {
error.FileNotFound => {
try self.makePath(sub_path);
return self.openIterableDir(sub_path, open_dir_options);
},
else => |e| return e,
};
},
};
}

/// This function returns the canonicalized absolute pathname of
Expand Down Expand Up @@ -1742,7 +1810,10 @@ pub const Dir = struct {
const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA |
w.SYNCHRONIZE | w.FILE_TRAVERSE;
const flags: u32 = if (iterable) base_flags | w.FILE_LIST_DIRECTORY else base_flags;
var dir = try self.openDirAccessMaskW(sub_path_w, flags, args.no_follow);
var dir = try self.makeOpenDirAccessMaskW(sub_path_w, flags, .{
.no_follow = args.no_follow,
.create_disposition = w.FILE_OPEN,
});
return dir;
}

Expand All @@ -1765,7 +1836,12 @@ pub const Dir = struct {
return Dir{ .fd = fd };
}

fn openDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32, no_follow: bool) OpenError!Dir {
const MakeOpenDirAccessMaskWOptions = struct {
no_follow: bool,
create_disposition: u32,
};

fn makeOpenDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32, flags: MakeOpenDirAccessMaskWOptions) OpenError!Dir {
const w = os.windows;

var result = Dir{
Expand All @@ -1786,21 +1862,22 @@ pub const Dir = struct {
.SecurityDescriptor = null,
.SecurityQualityOfService = null,
};
const open_reparse_point: w.DWORD = if (no_follow) w.FILE_OPEN_REPARSE_POINT else 0x0;
const open_reparse_point: w.DWORD = if (flags.no_follow) w.FILE_OPEN_REPARSE_POINT else 0x0;
var io: w.IO_STATUS_BLOCK = undefined;
const rc = w.ntdll.NtCreateFile(
&result.fd,
access_mask,
&attr,
&io,
null,
0,
w.FILE_ATTRIBUTE_NORMAL,
w.FILE_SHARE_READ | w.FILE_SHARE_WRITE,
w.FILE_OPEN,
flags.create_disposition,
w.FILE_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT | w.FILE_OPEN_FOR_BACKUP_INTENT | open_reparse_point,
null,
0,
);

switch (rc) {
.SUCCESS => return result,
.OBJECT_NAME_INVALID => return error.BadPathName,
Expand Down
28 changes: 20 additions & 8 deletions lib/std/fs/path.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1507,6 +1507,14 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
/// For example, if the path is `/a/b/c` and the most recently returned component
/// is `b`, then this will return the `c` component.
pub fn next(self: *Self) ?Component {
const peek_result = self.peekNext() orelse return null;
self.start_index = peek_result.path.len - peek_result.name.len;
self.end_index = peek_result.path.len;
return peek_result;
}

/// Like `next`, but does not modify the iterator state.
pub fn peekNext(self: Self) ?Component {
var start_index = self.end_index;
while (start_index < self.path.len and path_type.isSep(T, self.path[start_index])) {
start_index += 1;
Expand All @@ -1516,11 +1524,9 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
end_index += 1;
}
if (start_index == end_index) return null;
self.start_index = start_index;
self.end_index = end_index;
return .{
.name = self.path[self.start_index..self.end_index],
.path = self.path[0..self.end_index],
.name = self.path[start_index..end_index],
.path = self.path[0..end_index],
};
}

Expand All @@ -1529,6 +1535,14 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
/// For example, if the path is `/a/b/c` and the most recently returned component
/// is `b`, then this will return the `a` component.
pub fn previous(self: *Self) ?Component {
const peek_result = self.peekPrevious() orelse return null;
self.start_index = peek_result.path.len - peek_result.name.len;
self.end_index = peek_result.path.len;
return peek_result;
}

/// Like `previous`, but does not modify the iterator state.
pub fn peekPrevious(self: Self) ?Component {
var end_index = self.start_index;
while (true) {
if (end_index == self.root_end_index) return null;
Expand All @@ -1542,11 +1556,9 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
start_index -= 1;
}
if (start_index == end_index) return null;
self.start_index = start_index;
self.end_index = end_index;
return .{
.name = self.path[self.start_index..self.end_index],
.path = self.path[0..self.end_index],
.name = self.path[start_index..end_index],
.path = self.path[0..end_index],
};
}
};
Expand Down
12 changes: 12 additions & 0 deletions lib/std/fs/test.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,18 @@ test "file operations on directories" {
}.impl);
}

test "makeOpenPath parent dirs do not exist" {
var tmp_dir = tmpDir(.{});
defer tmp_dir.cleanup();

var dir = try tmp_dir.dir.makeOpenPath("root_dir/parent_dir/some_dir", .{});
dir.close();

// double check that the full directory structure was created
var dir_verification = try tmp_dir.dir.openDir("root_dir/parent_dir/some_dir", .{});
dir_verification.close();
}

test "deleteDir" {
try testWithAllSupportedPathTypes(struct {
fn impl(ctx: *TestContext) !void {
Expand Down

0 comments on commit e354aac

Please sign in to comment.