std.tar: add writer #19603
Merged
std.tar: add writer #19603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you share the performance difference before/after? |
|
Generating sources.tar 10 times in the loop: before: after: The difference is because I used smaller buffer when writing file content 512 bytes instead of 4000 as in any().writeFile. Let me do something about that. |
ianic
added a commit
to ianic/zig
that referenced
this pull request
Apr 12, 2024
Fixing performance problem: ziglang#19603 (comment)
ianic
added a commit
to ianic/zig
that referenced
this pull request
Apr 12, 2024
Fixing performance problem: ziglang#19603 (comment)
ianic
added a commit
to ianic/zig
that referenced
this pull request
Apr 12, 2024
Fixing performance problem: ziglang#19603 (comment)
|
Comparing previous and this in creating sources.tar Code used for benchmark: prevpub fn main() !void {
var gpa_instance = std.heap.GeneralPurposeAllocator(.{}){};
defer std.debug.assert(gpa_instance.deinit() == .ok);
const gpa = gpa_instance.allocator();
var lib_dir = try std.fs.cwd().openDir("/home/ianic/Code/zig/lib", .{});
var out_dir = try std.fs.cwd().openDir("/home/ianic/Code/tmp", .{});
// previous version
for (0..10) |_| {
var out_file = try out_dir.createFile("sources.tar", .{});
defer out_file.close();
var w = out_file.writer();
var std_dir = try lib_dir.openDir("std", .{ .iterate = true });
defer std_dir.close();
var walker = try std_dir.walk(gpa);
defer walker.deinit();
while (try walker.next()) |entry| {
switch (entry.kind) {
.file => {
if (!std.mem.endsWith(u8, entry.basename, ".zig"))
continue;
if (std.mem.endsWith(u8, entry.basename, "test.zig"))
continue;
},
else => continue,
}
var file = try std_dir.openFile(entry.path, .{});
defer file.close();
const stat = try file.stat();
const padding = p: {
const remainder = stat.size % 512;
break :p if (remainder > 0) 512 - remainder else 0;
};
var file_header = std.tar.output.Header.init();
file_header.typeflag = .regular;
try file_header.setPath("std", entry.path);
try file_header.setSize(stat.size);
try file_header.updateChecksum();
try w.writeAll(std.mem.asBytes(&file_header));
try w.any().writeFile(file);
try w.writeByteNTimes(0, padding);
}
{
// Since this command is JIT compiled, the builtin module available in
// this source file corresponds to the user's host system.
const builtin_zig = @embedFile("builtin");
var file_header = std.tar.output.Header.init();
file_header.typeflag = .regular;
try file_header.setPath("builtin", "builtin.zig");
try file_header.setSize(builtin_zig.len);
try file_header.updateChecksum();
try w.writeAll(std.mem.asBytes(&file_header));
try w.writeAll(builtin_zig);
const padding = p: {
const remainder = builtin_zig.len % 512;
break :p if (remainder > 0) 512 - remainder else 0;
};
try w.writeByteNTimes(0, padding);
}
}
}
thispub fn main() !void {
var gpa_instance = std.heap.GeneralPurposeAllocator(.{}){};
defer std.debug.assert(gpa_instance.deinit() == .ok);
const gpa = gpa_instance.allocator();
var lib_dir = try std.fs.cwd().openDir("/home/ianic/Code/zig/lib", .{});
var out_dir = try std.fs.cwd().openDir("/home/ianic/Code/tmp", .{});
for (0..10) |_| {
var out_file = try out_dir.createFile("sources_new.tar", .{});
defer out_file.close();
var w = std.tar.writer(out_file.writer().any());
try w.setRoot("std");
var std_dir = try lib_dir.openDir("std", .{ .iterate = true });
defer std_dir.close();
var walker = try std_dir.walk(gpa);
defer walker.deinit();
while (try walker.next()) |entry| {
switch (entry.kind) {
.file => {
if (!std.mem.endsWith(u8, entry.basename, ".zig"))
continue;
if (std.mem.endsWith(u8, entry.basename, "test.zig"))
continue;
},
else => continue,
}
var file = try entry.dir.openFile(entry.basename, .{});
defer file.close();
try w.writeFile(entry.path, file);
}
{
// Since this command is JIT compiled, the builtin module available in
// this source file corresponds to the user's host system.
const builtin_zig = @embedFile("builtin");
w.prefix = "builtin";
try w.writeFileBytes("builtin.zig", builtin_zig, .{});
}
}
}There were two use cases which pushed me into creating tar.writer:
I started modifying output.Header to support tarballs without prefix. Then found a file while creating tarball of the zig source, which has too long name. Realized that we need to support pax header or gnu long names. Wanted to have api where user don't need to think about adding checksum to the header, adding padding at file content. So instead of modifying output.Header moved all that to tar.writer. |
|
The computer is doing a lot more work than before. I have a strong suspicion that a different API could result in equivalent performance, while still providing the desired abstraction. |
|
This version is also setting mode and mtime for tar files. Not needed in this case. I removed that and did some small optimization. |
ianic
added a commit
to ianic/zig
that referenced
this pull request
Apr 13, 2024
Fixing performance problem: ziglang#19603 (comment)
|
I did few optimizations. |
andrewrk
approved these changes
Aug 12, 2024
Current `setPath` doesn't handle non prefix cases.
So we can write filenames (and links) of arbitrary length.
Move writer to tar/writer.zig
Compare output of writer with current version.
Replaced with writer.
/home/ci/actions-runner1/_work/zig/zig/lib/std/tar/writer.zig:112:54: error: expected type 'usize', found 'u64' /home/ci/actions-runner1/_work/zig/zig/lib/std/tar/writer.zig:112:54: note: unsigned 32-bit int cannot represent all possible unsigned 64-bit values /home/ci/actions-runner1/_work/zig/zig/lib/std/tar/writer.zig:54:65: note: parameter type declared here
Fixing performance problem: ziglang#19603 (comment)
Use AnyWriter.writeFile if file system file is provided to the writer.
Init Header with file defaults. Writing file is most common case. Conversion to octal without bufPrint. Checksum calculation without branching.
|
Thanks for the rebase :) |
richerfu
pushed a commit
to richerfu/zig
that referenced
this pull request
Oct 28, 2024
Simplifies code in docs creation where we used `std.tar.output.Header`. Writer uses that Header internally and provides higher level interface. Updates checksum on write, handles long file names, allows setting mtime and file permission mode. Provides handy interface for passing `Dir.WalkerEntry`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simplifies code in docs creation where we used
std.tar.output.Header.Writer uses that Header internally and provides higher level interface.
Updates checksum on write, handles long file names, allows setting mtime and file permission mode. Provides handy interface for passing Dir.WalkerEntry.
Tested that
zig stdandzig test -femit-docsare creatingsources.taras before this change.