<adamkowalski>
that needs memory to allocate the tensor and copies the data being passed
<adamkowalski>
so it uses the arena which was defined at the top of the scope
<adamkowalski>
the reason is sometimes you have many expected values and you don't want to have to write defer expected0.deinit() for each one
<adamkowalski>
so the arena is purely for the expected values at the bottom of the tests where the assertions happen
<adamkowalski>
it just happens to be overkill in this case, but I favored consistency between all my tests rather than just using the page allocator here and arena elsewhere
<jjido>
I strongly feel there should only be one allocator in that function
<adamkowalski>
in which function
<adamkowalski>
you mean the unit test?
<jjido>
yes the test function in this case, but it can be any function using your library
<adamkowalski>
but i'm open to suggestions, wanna create a pastebin with your proposal?
<adamkowalski>
are you saying remove the arena and have everyone use the heap.page_allocator
<jjido>
can't you create an arena allocator out of another arena allocator? That may be appropriate for Session
<adamkowalski>
Thanks for taking the time to write that up
<adamkowalski>
So for your second idea, the graph does handle all the allocation, it has an internal arena and everytime you add an operation it will allocate using that allocator
<adamkowalski>
same thing with session, it uses it's own arena
<adamkowalski>
however, the hitch is eager.constant
<adamkowalski>
that is a whole seperate namespace which contains "eager" versions of all the operations
<adamkowalski>
so lazy add will actually shell out to eager add when the time comes to do work
<adamkowalski>
eager operations have no notions of the graph and only need allocators to do their work
<jjido>
that's why I suggested using "graph" as some kind of allocator
<adamkowalski>
it is
<adamkowalski>
but eager operations are not meant to be used by consumers of the api
<jjido>
ignoring it is a graph
<adamkowalski>
well I suppose you can call &graph.allocator
<adamkowalski>
but that ties the lifetime of the eager constant and the graph
<jjido>
sounds fine
<jjido>
it's a convenience, let the use choose
<adamkowalski>
the graph, session, and any eager tensors should have disjoint lifetimes
<adamkowalski>
However, I agree in this test code you could do that and it would be fine
<adamkowalski>
it would make the code a little simpler
<adamkowalski>
however, I just realized an issue
<adamkowalski>
now I'm forced to expose the guts of the graph to the user, and if they call &graph.allocator I'm forced to keep that as part of the api when it is supposed to be an implementation detail
<adamkowalski>
I can now never change the name of that field or change how I allocate without breaking users code
<adamkowalski>
I really think graph allocations should only deal with things that will be part of the graph
<jjido>
ok
<adamkowalski>
However, I do agree that the current code should be slimmed down and made more sexy, maybe use less allocators and become more streamlined
<jjido>
you said that Graph creates an arena allocator
<adamkowalski>
I just want to think about how I approach that so that graphs are their own thing with their own lifetime and they don't get mixed up with anything else
<adamkowalski>
yes internally they do, and you shouldn't know that
<jjido>
so graph.allocator gives you an allocator tied to the lifetime of the graph
<adamkowalski>
well not atm because the allocator is not a public field
<adamkowalski>
and I'm not convinced I want people monkying around with the internals of the graph and/or relying on them
<adamkowalski>
it breaks encapsulation
<adamkowalski>
but yes the graph has an allocator by that name
<jjido>
as long as "graph.allocator" means an allocator tied to the lifetime of the graph, it doesn't matter if you change the way graph allocates - you can just use that something else and still provide the old graph.allocator in the API
<jjido>
I think it makes the code look cleaner
<adamkowalski>
okay, but what about your first proposal
<adamkowalski>
I liked that one too
<jjido>
don't like to expose that much "piping"
<jjido>
yeah if that works, it's good too
<adamkowalski>
since the allocator is in scope at the top level, why not just use that
<adamkowalski>
I could make that even simpler and not even use an arena
<adamkowalski>
just pass the heap allocator to the graph and the session directly
<jjido>
if you do that what about eager.constant?
<adamkowalski>
defer deinit that sucker
<adamkowalski>
but yes that is the ugly duckling left
<adamkowalski>
but actually talking about this with you helped me realize a flaw in the session allocation strategy
<adamkowalski>
the returned value from session.run is tied to the lifetime of the session
<adamkowalski>
since it uses the sessions arena
<adamkowalski>
they should probably be disjoint
<jjido>
the arena is not really the issue, it's having more than one allocator around which is
<adamkowalski>
you mean in general or just in this particular example?
<jjido>
yes the returned value should use the initial allocator
<jjido>
this particular example
<adamkowalski>
yeah okay I'm sold I'll make the change
jjido has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
<adamkowalski>
now the question is how to approach the session.run
adamkowalski has quit [Quit: Lost terminal]
_Vi has quit [Ping timeout: 240 seconds]
adamkowalski has joined #zig
_whitelogger has joined #zig
adamkowalski has quit [Quit: Lost terminal]
_whitelogger has joined #zig
Kingsquee has joined #zig
daex has quit [Ping timeout: 240 seconds]
daex has joined #zig
waleee-cl has quit [Quit: Connection closed for inactivity]
adamkowalski has joined #zig
daex_ has joined #zig
daex has quit [Ping timeout: 246 seconds]
daex has joined #zig
daex_ has quit [Ping timeout: 246 seconds]
daex_ has joined #zig
daex has quit [Ping timeout: 250 seconds]
adamkowalski has quit [Quit: Lost terminal]
ur5us has quit [Ping timeout: 246 seconds]
_whitelogger has joined #zig
<pixelherodev>
LLVM IR parsing and AST gen of simple test file: 0.06 seconds in debug mode, `assertion failed in ir.cpp` in release-fast mode
<pixelherodev>
:P
<pixelherodev>
And in release-small
dddddd has quit [Ping timeout: 265 seconds]
_whitelogger has joined #zig
_Vi has joined #zig
tdog has joined #zig
ur5us has joined #zig
_Vi has quit [Ping timeout: 240 seconds]
ur5us has quit [Ping timeout: 246 seconds]
dingenskirchen has quit [Quit: dingenskirchen]
dingenskirchen has joined #zig
_Vi has joined #zig
mokafolio has quit [Quit: Bye Bye!]
mokafolio has joined #zig
squeek502 has joined #zig
squeek_ has joined #zig
squeek502_ has quit [Ping timeout: 260 seconds]
squeek502 has quit [Ping timeout: 246 seconds]
urluck has quit [Ping timeout: 272 seconds]
urluck has joined #zig
waleee-cl has joined #zig
ifreund has joined #zig
squeek502_ has joined #zig
Kingsquee has quit [Quit: Konversation terminated!]
squeek_ has quit [Ping timeout: 246 seconds]
marijnfs has joined #zig
marijnfs has quit [Quit: Lost terminal]
squeek502 has joined #zig
squeek502_ has quit [Ping timeout: 256 seconds]
marijnfs has joined #zig
marijnfs has quit [Quit: Lost terminal]
marijnfs has joined #zig
dddddd has joined #zig
adamkowalski has joined #zig
Pursche01 has joined #zig
marijnfs has quit [Ping timeout: 256 seconds]
zfoo_ has joined #zig
marijnfs has joined #zig
zfoo_ has quit [Ping timeout: 246 seconds]
<shakesoda>
build on the site is from the 12th, what happened
* shakesoda
had to manually update fs.zig from master due to some breakage in that build
TheLemonMan has joined #zig
marijnfs has quit [Ping timeout: 246 seconds]
<tdeo>
playing with async and can't seem to find a way for an async function to remove its frame from a list on its own, noasync should theoretically be able to solve this, right (it doesn't now)? https://tadeo.ca/u/durBI.png
recombinant has joined #zig
<tdeo>
though i guess since std.event.Loop is using a threadpool anyway this way won't work
_Vi has quit [Ping timeout: 246 seconds]
<andrewrk>
shakesoda, I noticed the download page has been lagging but I'm not sure what's going wrong in the CI yet
<andrewrk>
one thing I noticed is that azure stopped running CI builds for merged PRs, which is problematic
recombinant has quit [Quit: Leaving]
recombinant has joined #zig
_Vi has joined #zig
<andrewrk>
hmm yeah I just rebase-merged a PR into master, and azure is not running
<andrewrk>
I don't even know how to troubleshoot this. azure just changed its behavior and broke stuff
<andrewrk>
tdeo, you may be able to work around this by using *Client rather than Client in the list
_Vi has quit [Ping timeout: 246 seconds]
<andrewrk>
it looks like either your memory layout actually does have a cycle, or zig's lazy value system needs an improvement to understand that this is not a cycle
<andrewrk>
I can almost tell from the stack trace but not quite
<shakesoda>
after fixing, like, a thousand things, i was able to work around #1481 and get cimgui working again even on linux
<shakesoda>
although i'd rather not have had to change so much code to do it
<andrewrk>
we'll get there, it will just take some time
<andrewrk>
nice work
<tdeo>
the problem with it is Client includes @Frame(run) where run calls deinit, so while analyzing deinit it doesn't seem to know what the size of Client is, so it can't use generic types based on it
<tdeo>
getting the same problem with std.atomic.Queue :(, i guess an extra indirection isn't too bad since it's rarely touched
<shakesoda>
andrewrk: zig improves at a healthy pace, i have no doubt that it will get there
<shakesoda>
i did run into a nasty bug in opt builds last night though, i had to mess with my fs read function
<andrewrk>
I kicked off a manual CI run of master, in theory this should update the download page
<andrewrk>
tdeo, btw I'm happy that you're trying out the event loop and async/await features, but also just as reminder, this is definitely "experimental" territory (although it's what I'm currently working on improving)
<tdeo>
yeah, i do understand that
<tdeo>
i'm really excited, the async io feels so much nicer than the situation on other systems languages
<tdeo>
i'm slowly working on writing a wayland compositor in zig, from scratch as much is reasonable, i've written the wire protocol part in pure zig
<tdeo>
probably will never finish but it's fun
<shakesoda>
meanwhile i have been suffering at the hands of the fs apis
<shakesoda>
https://godbolt.org/z/bG4nNA with this function in release-fast builds and without @setRuntimeSafety(true) in the block, it reads back a corrupted disaster
<andrewrk>
shakesoda, what os?
<shakesoda>
this is almost certainly bad code, but it behaves completely differently in fast vs safe modes
<shakesoda>
andrewrk: x86_64 linux
<shakesoda>
i believe it works fine on x64 windows
* shakesoda
checks
<andrewrk>
shakesoda, I think you want to pass `path` to `dirname`
<andrewrk>
you're passing the entire buffer
<shakesoda>
oh, good spot, that's definitely a typo
<andrewrk>
unfortunately not everything can be caught by safety features, and this can lead to different behavior in different build modes
<shakesoda>
andrewrk: good spot, that was the bug, thank you
<andrewrk>
debug mode initializes undefined values to 0xaaaaa, so I would actually expect debug mode to fail harder
<andrewrk>
shakesoda, one thing you can try, if you don't mind going back to the old (broken) code - run it in strace and look for aaaaaa
<andrewrk>
if you see that, you know you passed undefined data to a syscall
<andrewrk>
also, try running it in valgrind, it should also identify the problem, with a stack trace even
<andrewrk>
I'm actually quite curious to know what the valgrind output is for the broken code in debug mode
<shakesoda>
andrewrk: the syscall is good in debug
<andrewrk>
no complaints from valgrind?
<shakesoda>
just checked strace
<shakesoda>
ugh, valgrind output is so incredibly noisy
<shakesoda>
so many complaints about the i965 driver
<shakesoda>
32000 lines of output before anything i might have done
<shakesoda>
andrewrk: I did indeed find a "param openat(filename) points to uninitialized byte(s)" in release mode, which would be the offender here
<shakesoda>
I've got no idea why in debug everything seems okay
Akuli has joined #zig
<andrewrk>
hmm that is indeed curious
<mikdusan>
wouldn't that behavior depend entirely on what cwd.openFile does with a bunch of newlines in filename?
<mikdusan>
or rather inverse. nulls.
so has quit [*.net *.split]
via has quit [*.net *.split]
bkleiner has quit [*.net *.split]
via has joined #zig
so has joined #zig
bkleiner has joined #zig
mahmudov has quit [Ping timeout: 260 seconds]
_Vi has joined #zig
<andrewrk>
ah, it was 2 things. the azure thing, but also update-download-page.zig needed to be updated to new streams API
pixelherodev has quit [Changing host]
pixelherodev has joined #zig
tdog has quit [Ping timeout: 256 seconds]
pixelherodev has quit [Changing host]
pixelherodev has joined #zig
ur5us has joined #zig
lqd has quit [Quit: Connection closed for inactivity]
lqd has joined #zig
FireFox317 has joined #zig
<FireFox317>
i'm using the addSystemCommand api in build.zig to run qemu, however stdin is not forwarded to the qemu command and thus im not possible to interact with it when doing `zig build qemu`. However when I run the qemu command in a terminal it works fine. Does someone know how to get the correct behavior?
<andrewrk>
FireFox317, the code is in lib/std/build/run.zig
<andrewrk>
you can see it unconditionally sets stdin to `.Ignore`. so this will have to be improved to support this use case
<FireFox317>
ah yes, i see thanks
<andrewrk>
FireFox317, if you look at StdIoAction, that should probably be renamed to StdIoOutputAction and then another enum introduced for stdin, with options such as ignore, input_this_slice, forward_terminal_stdin, etc. make sense?
<andrewrk>
perhaps the default ought to be `inherit`
<andrewrk>
rather than `ignore`
<FireFox317>
yeah i guess that makes more sense, because when you are running a system command, you also expect to be able to use the stdin and stdout of that command
<FireFox317>
well, thats what i would expect
<andrewrk>
makes sense to me
<andrewrk>
I'm happy to merge a PR if you tell me it's working for your project. standalone tests would be welcome but not required for this
<andrewrk>
(standalone tests being things in the test/standalone/ directory
marijnfs has joined #zig
<FireFox317>
andrewrk: I just changed line 162 of run.zig to default to inherit and it works. Is that enough for a PR or do you want me to do the StdIoAction thing you said before?
<andrewrk>
oh I see. hmm. well I'm ok with changing the default, but since that's a breaking change, I do want there to be an answer to the question, "how do I update my code to get the previous behavior?"
<andrewrk>
so I think a minimal mergeable PR would be to have that enum with inherit, or ignore, configurable as a field on RunStep
<danyspin97>
quit
<danyspin97>
ops lol
<danyspin97>
btw, zig has been successfully packaged for exherbo :)
mahmudov has joined #zig
<FireFox317>
andrewrk: #4751
<andrewrk>
FireFox317, thanks. and it works for you, yeah?
<FireFox317>
Yes definitely
<FireFox317>
andrewrk, Thanks!
Akuli has quit [Quit: Leaving]
frmdstryr has joined #zig
<andrewrk>
this week in llvm: * The AVR backend was promoted to a standard backend, built by default.
<andrewrk>
note, that applies not to the upcoming llvm 10 but to llvm 11
<mikdusan>
neat
TheLemonMan has quit [Quit: "It's now safe to turn off your computer."]
<pixelherodev>
Thanks for fixing the build.zig problem!
FireFox317 has quit [Quit: Leaving]
jmiven has quit [Quit: bye]
Pursche01 has quit [Quit: Connection closed for inactivity]
jmiven has joined #zig
<pixelherodev>
I think the source_filename generated by Zig is wrong
<pixelherodev>
From the LLVM LangRef, it looks like that should be the absolute path to the file
<pixelherodev>
But it just contains the file name without extension
<pixelherodev>
I'd have just submitted a patch, but I'm not actually sure if I'm correct here :P
recombinant has quit [Read error: Connection reset by peer]