Matthew Heath (Dec 10 2024 at 22:22): Matthew Heath (Dec 10 2024 at 22:22):One attempt I have made in trying to expose the functionality of the Zig SHA256 algorithm in Roc is as a struct containing a pointer to the Zig struct , which I allocate with utils.allocateWithRefcount.
https://github.com/MatthewJohnHeath/rocSHA/blob/main/crates/compiler/builtins/bitcode/src/crypto.zig
My initial mental model was that something, somewhere in the compiler would be tracking this and I could think of it like a RAII smart pointer that would be cleaned up automatically when the struct was released in a Roc program.
I think I had this wrong and may be leaking memory. Could someone please give me some pointers on what does and doesn't happen to the values at pointers inside a struct when the struct is released.?
Brendan Hansknecht (Dec 10 2024 at 22:26):Probably want: Sha256 := { location : Box {} }
Brendan Hansknecht (Dec 10 2024 at 22:27):That will lead to roc generating code to refcount
Matthew Heath (Dec 10 2024 at 22:32):Thank you. Am I right in thinking that makes allocating with the ref count wrong and/or redundant here? Should it just be a direct call to roc_alloc on the Zig side and the Box in the Roc handles the lifetime?
Brendan Hansknecht (Dec 10 2024 at 23:03):You still need to allocate with refcount on the zig side. Cause zig is returning the box to roc. So roc will assume the box is fully setup with refcount.
Brendan Hansknecht (Dec 10 2024 at 23:04):Oh also, this is probably wrong: defer std.testing.allocator.destroy(empty_sha.location);
Brendan Hansknecht (Dec 10 2024 at 23:04):Cause the location points to the data, but the allocation and refcount is before the data
Brendan Hansknecht (Dec 10 2024 at 23:05):would be better to defer utils.decref(...)
Brendan Hansknecht (Dec 10 2024 at 23:05):this is just in a zig test
Brendan Hansknecht (Dec 10 2024 at 23:06):Also, in the medium term (or part of this pr if you want), we should be able to optimize sha256AddBytes. If the refcount is unique, it can reuse the allocation.
Brendan Hansknecht (Dec 10 2024 at 23:07):Oh, and I think that sha256AddBytes probably needs to defer utils.decref(...) the input or it will be leaked. Though that depends on what you told the builtin mapping around ownership vs borrowing.
Brendan Hansknecht (Dec 11 2024 at 17:09):@Matthew Heath was this enough info for you to figure out your memory issues?
Matthew Heath (Dec 11 2024 at 17:55):@Brendan Hansknecht I have been asleep and then at work, but I think it will be :)
Matthew Heath (Dec 11 2024 at 17:55):Thanks
Brendan Hansknecht (Dec 11 2024 at 17:59):Haha, no worries. Some reason, I thought I posted this longer ago and forgot to loop back.
Matthew Heath (Dec 11 2024 at 18:05):Brendan Hansknecht said:
Cause the location points to the data, but the allocation and refcount is before the data
As it's currently written, the allocation when testing is just calling std.testing.allocator.create because I wrote before I fixed the alignment of the rtesting_roc_alloc, so destroy was right (I think), but won't be after I change anything.
Richard Feldman (Dec 11 2024 at 18:14):perhaps unrelated, but shouldn't sha256 be able to run without heap allocations? :big_smile:
Matthew Heath (Dec 11 2024 at 18:28):Richard Feldman said:
perhaps unrelated, but shouldn't sha256 be able to run without heap allocations? :big_smile:
Yes the Zig built-in does, and single "hash these bytes" function easily could, but passing the struct representing the hasher with some bytes added to it between Roc functions is proving challenging.
After a lot of attempts, I worked out how to write the bytes of a non-extern Zig struct onto something exposed as extern and it worked as expected for Zig tests, but calling it from Roc generated llvm errors.
Brendan Hansknecht (Dec 11 2024 at 18:30):How big is the state in practice? Just 256 bits?
Brendan Hansknecht (Dec 11 2024 at 18:36):If my estimate is right, 112 bytes in zig. So 7 I128 in size. That probably should be on the heap in roc. Cause it will want to be mutated in place and not copied all the time
Brendan Hansknecht (Dec 11 2024 at 18:37):If it is made into a struct of 7 I128, it would be put on the stack, but it would also be copied for every modification.
Matthew Heath (Dec 11 2024 at 18:38):Brendan Hansknecht said:
If my estimate is right, 112 bytes in zig. So 7 I128 in size. That probably should be on the heap in roc. Cause it will want to be mutated in place and not copied all the time
Ah you got ahead of me :). @sizeOF actually gives 128 because of padding to align
Brendan Hansknecht (Dec 11 2024 at 18:38):So 8 I128s
Brendan Hansknecht (Dec 11 2024 at 18:39):Yeah, my gut feeling is that this needs to be refcounted and mutated in place.
Matthew Heath (Dec 11 2024 at 18:41):Brendan Hansknecht said:
If it is made into a struct of 7 I128, it would be put on the stack, but it would also be copied for every modification.
Yes, my reasoning was wanting to avoid that. But also when I tried to return any struct or array with more than 2 fields from Zig to Roc, it was building but then it said llvm didn't return a value. I was going to make a separate thread to ask about it when I had dealt with this....
Brendan Hansknecht (Dec 11 2024 at 18:42):Makes sense. Navigating that can be a bit tricky
Last updated: Jul 06 2025 at 12:14 UTC