std.os read/write functions + sendfile #4611

andrewrk · 2020-03-03T07:28:01Z

This merges #4131 and then does the following:

rework os.sendfile and add macosx support, and a fallback
implementation for any OS.
fix sendto compile error
std.os write functions support partial writes. closes std.os.writev does not handle partial writes #3443.
std.os pread / pwrite functions can now return error.Unseekable.
std.fs.File read/write functions now have readAll/writeAll variants
which loop to complete operations even when partial reads/writes
happen.
Audit std.os read/write functions with respect to Linux returning
EINVAL for lengths greater than 0x7fff0000.
std.os read/write shim functions do not unnecessarily loop. Since
partial reads/writes are part of the API, the caller will be forced
to loop anyway, and so that would just be code bloat.
Improve doc comments
Add a non-trivial test for std.os.sendfile
Fix std.os.pread on 32 bit Linux
Add missing SYS_sendfile bit on aarch64

This changset adds a `sendfile(2)` syscall bindings to the linux bits component. Where available, the `sendfile64(2)` syscall will be transparently called. A wrapping function has also been added to the std.os to transform errno returns to Zig errors. Change-Id: I86769fc4382c0771e3656e7b21137bafd99a4411

* rework os.sendfile and add macosx support, and a fallback implementation for any OS. * fix sendto compile error * std.os write functions support partial writes. closes #3443. * std.os pread / pwrite functions can now return `error.Unseekable`. * std.fs.File read/write functions now have readAll/writeAll variants which loop to complete operations even when partial reads/writes happen. * Audit std.os read/write functions with respect to Linux returning EINVAL for lengths greater than 0x7fff0000. * std.os read/write shim functions do not unnecessarily loop. Since partial reads/writes are part of the API, the caller will be forced to loop anyway, and so that would just be code bloat. * Improve doc comments * Add a non-trivial test for std.os.sendfile * Fix std.os.pread on 32 bit Linux * Add missing SYS_sendfile bit on aarch64

daurnimator · 2020-03-03T07:37:23Z

lib/std/os.zig

-            if (amt_read == 0) return off; // EOF
-        } else unreachable; // TODO https://github.com/ziglang/zig/issues/707
+    if (std.Target.current.os.tag == .windows) {
+        // TODO does Windows have a way to read an io vector?


yes. ReadFileScatter

Thanks- I will open an issue

daurnimator · 2020-03-03T07:38:54Z

lib/std/os.zig

-        }
+pub fn preadv(fd: fd_t, iov: []const iovec, offset: u64) PReadError!usize {
+    const have_pread_but_not_preadv = switch (std.Target.current.os.tag) {
+        .windows, .macosx, .ios, .watchos, .tvos => true,


windows has NtReadFileScatter

daurnimator · 2020-03-03T07:39:32Z

lib/std/os.zig

+/// If `iov.len` is larger than will fit in a `u31`, a partial write will occur.
+pub fn writev(fd: fd_t, iov: []const iovec_const) WriteError!usize {
+    if (std.Target.current.os.tag == .windows) {
+        // TODO does Windows have a way to write an io vector?


WriteFileGather

daurnimator · 2020-03-03T07:41:46Z

lib/std/os.zig

+    }
+
+    rw: {
+        var buf: [8 * 4096]u8 = undefined;


Should use a LinearFifo here

It lets you gracefully handle partial reads and partial writes (which helps in both speed, number of syscalls and correctness)

This code is intentionally doing exactly one read/write and returning a partial write. The burden of proof is on you to show why LinearFifo would be better than this.

ah sorry, I thought there was a loop here.
That'll come in the fallback path for copy_file_range later :)

Even copy_file_range can return a partial write, so I'm not sure that's true- the loopy code is in std.fs.File readAll() and writeAll() related functions. And std.io.OutStream.write.

Wait... we do need a loop here. A partial read (non 0) should be followed by another read right?

Nope, any looping is redundant since the function can return partial writes, and so the callsite must do looping or itself return partials. In fact, it's actually providing more power and information to return on the first partial read or partial write, since that exposes how the underlying mechanism works. As an example, this is exploited by command line input reading a line from the terminal.

daurnimator · 2020-03-03T07:47:11Z

lib/std/os/bits/linux/arm64.zig

@@ -82,6 +82,7 @@ pub const SYS_pread64 = 67;
 pub const SYS_pwrite64 = 68;
 pub const SYS_preadv = 69;
 pub const SYS_pwritev = 70;
+pub const SYS_sendfile = 71;


o.o how odd that this was missed!

yeah... i made some confused noises out loud when I saw that, lol

daurnimator · 2020-03-03T07:51:53Z

lib/std/os.zig

+        const amt_read = try pread(in_fd, buf[0..adjusted_count], in_offset);
+        if (amt_read == 0) {
+            if (count == 0) {
+                // We have detected EOF from `in_fd`.


I think we should still write 0 bytes here.

This would have an effect if e.g. the previous "write" was a sendto without MSG_EOR

I think there's value in considering this. Can you make that a separate proposal? I think all the read/write functions would need to be audited if this were the policy.

andrewrk · 2020-03-03T07:54:32Z

🤦‍♂️ CI is failing because / in branch name

daurnimator · 2020-03-03T07:58:03Z

Will also fail on freebsd due to upstream bug (see my other pr)

andrewrk · 2020-03-03T07:58:04Z

=> #4612

terinjokes and others added 2 commits March 2, 2020 12:54

andrewrk mentioned this pull request Mar 3, 2020

std: implement sendfile on linux #4131

Closed

daurnimator suggested changes Mar 3, 2020

View reviewed changes

daurnimator reviewed Mar 3, 2020

View reviewed changes

andrewrk closed this Mar 3, 2020

andrewrk deleted the terinjokes-patches/sendfile branch March 3, 2020 07:58

Uh oh!

std.os read/write functions + sendfile #4611

std.os read/write functions + sendfile #4611

Uh oh!

Conversation

andrewrk commented Mar 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewrk Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewrk commented Mar 3, 2020

Uh oh!

daurnimator commented Mar 3, 2020

Uh oh!

andrewrk commented Mar 3, 2020

Uh oh!

Uh oh!

andrewrk Mar 3, 2020 •

edited

Loading