zigzap / zap

blazingly fast backends in zig
MIT License
1.99k stars 71 forks source link

Broken demo wrk_zigstd #18

Closed kamidev closed 1 year ago

kamidev commented 1 year ago

I ran through the demos on an M2 Max today, the one below still fails. Haven't looked at it more carefully, but thought I should mention it.

➜  zap git:(master) zig build run-wrk_zigstd -fno-summary -freference-trace
zig build-exe wrk_zigstd Debug native: error: the following command failed with 1 compilation errors:
/Users/jonas/src/zig/zig/build/stage3/bin/zig build-exe -freference-trace=256 /Users/jonas/src/zig/zap/wrk/zigstd/main.zig /Users/jonas/src/zig/zap/zig-cache/o/d3a562268d75faad3020a7a59f13d9cb/libfacil.io.a -lc --cache-dir /Users/jonas/src/zig/zap/zig-cache --global-cache-dir /Users/jonas/.cache/zig --name wrk_zigstd --mod zap::/Users/jonas/src/zig/zap/src/zap.zig --deps zap -I /Users/jonas/src/zig/zap/zig-cache/i/88557c3f68f5d33d5605fe5573541278/include --listen=-
wrk/zigstd/main.zig:14:41: error: no field named 'dynamic' in struct 'http.Server.AcceptOptions'
        var res = try server.accept(.{ .dynamic = max_header_size });
                                        ^~~~~~~
/Users/jonas/src/zig/zig/build/stage3/lib/zig/std/http/Server.zig:733:27: note: struct declared here
pub const AcceptOptions = struct {
                          ^~~~~~
referenced by:
    comptime_0: /Users/jonas/src/zig/zig/build/stage3/lib/zig/std/start.zig:63:50
renerocksai commented 1 year ago

sorry I replied to wrong issue #14 .

This is fixed in 0c18565

Thx for pointing it out.

kamidev commented 1 year ago

For me, it doesn't finish compiling. It hangs here:

➜  zap git:(master) zig build run-wrk_zigstd
steps [26/28] zig build-exe wrk_zigstd Debug native... LLVM Emit Object...
kamidev commented 1 year ago

Is that the expected outcome? Here is my setup:

➜  zap git:(master) zig version; sw_vers; clang --version
0.11.0-dev.3220+447a30299
ProductName:        macOS
ProductVersion:     13.4
BuildVersion:       22F66
Homebrew clang version 16.0.4
Target: arm64-apple-darwin22.5.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
➜  zap git:(master) git log -1 | tee
commit 0c18565cea2b31df688ff89c09a815acfd4be273
Author: Rene Schallner <rene@renerocks.ai>
Date:   Fri May 19 15:37:58 2023 +0200

    fixed wrk_zigstd. It compiles but can't handle wrk :(

All other demo apps run without obvious problems.

renerocksai commented 1 year ago

It finishes compiling and starts running. It just doesn't print anything to the console. Have you checked the source???

This is not a zap example / demo, it is like the rust, python, go projects: only used in the wrk measurements.

It is not part of examples/

Why are you so obsessed with it? It uses std.http server to send a fixed response and makes no sense running on its own.

renerocksai commented 1 year ago

If you wanted to build it you would zig build wrk_zigstd. But building run-wrk_zigstd also runs it after building. Fyi

kamidev commented 1 year ago

Thanks. No obsession, I was just a bit too quick. Your code is straightforward but zig is moving fast, so I was basically scanning for build problems. Checking if tests and examples compiled and if the output seemed to match the code. After the last fixes, everything ran. Except one file seemed to hang, so I - incorrectly - assumed 1. it was an example and 2. it didn't build like the others. Sorry about that.

renerocksai commented 1 year ago

No worries. And thx for the QA check you did there. I didn't understand where you were coming from at first. The wrk_zigstd is / might one day become part of the perf measurement suite. As such, it's important that it compiles. I don't like stale code in a project. A while ago I set up a CI cronjob that runs every day, builds all executables, and runs all tests with the then-current zig master. To ensure std lib / zig changes I'm unaware of don't break things w/o me noticing. What I hadn't thought about was wrk_zigstd though. So, thx to your scanning job, all buildable code is now checked every day (and every commit).