add Io.Writer.AllocatingAligned #25050
Conversation
|
realized this should likely be named |
|
I'll leave this as-is since all ci checks passed and I'd prefer to have feedback before triggering another run. But I have those 2 name changes ready to go if wanted. |
this commit makes it easier to write to an aligned array list.
previously you needed to use an adapter such as
`aligned_list.writer(allocator).adaptToNewApi(&.{});`.
`Io.Writer.Allocating` becomes an `AllocatingAligned(.fromByteUnits(1))`
similar to how `std.ArrayList(u8)` is a
`std.array_list.Aligned(u8, null)`.
* I've turned off zig fmt to make this diff easier to read. Next commit
will be correctly formatted.
added a test to check for compile errors and one to check a few alignments. adding the second test exposed the deinit error i.e. Allocation alignment X does not match free alignment 1.
travisstaloch
force-pushed
the
io-writer-aligned-allocating
branch
from
8084f56 to
9b9ac5d
Compare
travisstaloch
changed the title
|
This needed a rebase anyway so just pushed the name changes and added a couple extra tests with different alignments. The new test errored with 'Allocation alignment X doesn't match free alignment 1' and showed that buffer needed an |
andrewrk
left a comment
andrewrk
left a comment
There was a problem hiding this comment.
Thanks for sending this! I agree we need something like this, although there are a few different ways of tackling the problem.
Since the buffer field will continue to be alignment-erased, perhaps a runtime-known alignment API is the way to go on this one.
I'd like to explore that option before committing to this generic version.
| return .{ | ||
| .allocator = allocator, | ||
| .writer = .{ | ||
| .buffer = try allocator.alloc(u8, capacity), |
There was a problem hiding this comment.
| .buffer = try allocator.alloc(u8, capacity), | |
| .buffer = try allocator.alignedAlloc(u8, alignment, capacity), |
Would you like me to work on that or would you like to? If me, do you have any suggestions about how to implement? I'm imagining perhaps an alignment field instead of a comptime parameter and type erased methods. Is that the direction you'd like to go? |
|
I'm trying it locally now. I think it's going to be a nice way to go |
|
Looks like you might have seen the relevant changes in #25077 already. Do you think it can address your use case as well? |
|
Yes my use case is covered by |
|
@andrewrk I think I'll open a follow up PR with tests that cover the new *Aligned methods with alignment greater than 1 unless you're confident they are covered elsewhere. |
|
I'd appreciate that! |
2 participants
this PR makes it easier to write to an aligned array list. previously you needed an adapter.