search

TooTallNate / ref

Turn Buffer instances into "pointers"
http://tootallnate.github.com/ref
453 stars 141 forks source link

ref.alloc ignores alignment requirements #28

Closed addaleax closed 9 years ago

addaleax commented 9 years ago

At least since the new Buffer system of node v0.12 has been introduced, new Buffer(size) does not necessarily result in a Buffer with any kind of alignment. Use of misaligned values often significantly impairs memory I/O perfomance and may result (depending on the circumstances) in crashes of the interpreter.

I was debugging a segmentation fault caused by this behaviour in a library using the ffi module and looked in the documention of ffi and ref for a safe way of allocating Buffers, and ref.alloc seems to be intended to be suitable for this purpose, e.g. I’m pretty sure ref.alloc(ref.types.double).address() % ref.types.double.alignment should always be 0.

I’m not really sure how this would be approached in the best possible way. I guess the naive solution which allocates alignment-1 bytes more than necessary and then returns an aligned slice would be fine (although there might be better ways). Also, in some situations it might be helpful to be able to override this from the JS side via an extra parameter to ref.alloc, for example, the current x86 SIMD extensions require alignments of 16 bytes and even more.

TooTallNate commented 9 years ago

Haven't forgotten about you here. Check out https://github.com/nodejs/io.js/pull/1750. Overall, I think the thing that makes this hard (at the moment) is that there's no way to introspect the memory address of a Buffer instance. If we had that, ref could alloc a big slice of memory and keep track of the memory address in order to alloc properly aligned buffer instances. The new Buffer#address() function introduced in that pull request above should make this possible.

TooTallNate commented 9 years ago

I think that Buffer is handling this in core at this point, and that the early v0.12 behavior was a bug. See https://github.com/nodejs/node/commit/07c066724ced98adcae3f9f97cfcd301b138eb18.

Please let me know if I'm mistaken, but gonna close this one for now.

addaleax commented 9 years ago

Sorry, your reply from May seems to have slipped by me – I guess you’re right, this pretty much looks like it fixes problems like this. Thank you!