-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Array#to_slice and Array#bytesize #4327
Conversation
src/array.cr
Outdated
# | ||
# This method is **unsafe** because when the array shrinks or grows, it might | ||
# change the position of the internal buffer. The slice would then point to an | ||
# uninialized memory region. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: uninitialized
Slices are supposed to be safe pointers. Sadly, exposing an Array's internal pointer as a slice is unsafe, because the internal pointer may be reallocated, and accessing any exported slice would segfault.
|
Maybe we could add |
|
What about this? Renamed |
IMHO a Slice ain't just a Pointer with a size, but also caries the idea of a safe struct. It's obvious a Pointer is unsafe, it's less obvious a Slice is, because we sell Slices as a safe alternative to Pointers. Adding You must be very careful with the exported, unsafe Slice, too; if you are writing the slice to an IO, the IO will likely switch fibers, and... if something mutates the Array in another Fiber: 💥 segfault. You may be confident when you write the code that this won't happen... but what about a few weeks or months later? when you optimize I support @RX14 solution: allocate and copy memory (that won't trigger the event loop). class Array(T)
def to_slice
pointer = Pointer(T).malloc(size)
pointer.copy_from(to_unsafe, size)
Slice(T).new(pointer, size)
end
end |
@ysbaddaden totally agree If we're going with the allocate & copy solution, the documentation should mention that it's slow (copy the array buffer), and even slower on an Array of struct. |
In that same vein, wouldn't it be better to be a little more explicit and call it |
Are you sure? |
@ysbaddaden It is not-obvious that a call to |
Last argument, then I shut up: naming should be done consistently on what is expected to be returned, not internal details optimizations. If a method could accept an Array, StaticArray, Slice of String or whatever, I would have to make a special case for Array, because it decided to use the unexpected Silly example: what about a |
|
Hi,
This PR adds
Array#to_slice
andArray#bytesize
including specs, and also adds a simple spec forArray#to_unsafe
.Use-cases are commonly found in I/O code, e.g., to quickly write an array of data into an
IO
:I think it's much better to offer a canonical way of turning an array into a slice for a moment, than having the user to fiddle with
ary.to_unsafe.to_slice(ary.size)
. It may not be obvious to some ifto_slice
in this case requires the byte size, or the count of elements.Sometimes you also just want to know the current size in memory of the whole array.
Array#bytesize
mirrorsSlice#bytesize
, and is much nicer to read thanary.size * sizeof(MyT)
, especially as the user has to figure out theT
. This also makes it easier to write generic code.Cheers!