Skip to content
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

ref.alloc ignores alignment requirements #28

Closed
addaleax opened this issue Mar 28, 2015 · 3 comments
Closed

ref.alloc ignores alignment requirements #28

addaleax opened this issue Mar 28, 2015 · 3 comments

Comments

@addaleax
Copy link

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.

addaleax added a commit to addaleax/node-lapack that referenced this issue Mar 28, 2015
This works around TooTallNate/ref#28
and may result in a general performance increase.
@TooTallNate
Copy link
Owner

Haven't forgotten about you here. Check out nodejs/node#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
Copy link
Owner

I think that Buffer is handling this in core at this point, and that the early v0.12 behavior was a bug. See nodejs/node@07c0667.

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

@addaleax
Copy link
Author

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!

addaleax added a commit to addaleax/node-lapack that referenced this issue Dec 12, 2015
This works around TooTallNate/ref#28
and may result in a general performance increase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants