buffer: align chunks on 8-byte boundary - #1126
Conversation
When slicing global pool - ensure that the underlying buffer's data ptr is 8-byte alignment to do not ruin expectations of 3rd party C++ addons. NOTE: 0.10 node.js always returned aligned pointers and io.js should do this too for compatibility.
024592c to
c7111d5
Compare
|
Suggested reviewer: @trevnorris @bnoordhuis |
There was a problem hiding this comment.
I'm curious, why is this line needed if the condition already succeeded?
There was a problem hiding this comment.
poolOffset = 1poolOffset & 0x7 === 1poolOffset |= 0x7=>0x7poolOffset++=>0x8- Voila,
poolOffsetis aligned!
There was a problem hiding this comment.
@mscdex in other words, I'm just setting all bits that are lower than alignment value and increment the number to align it.
There was a problem hiding this comment.
A less convoluted way of writing that is poolOffset = (poolOffset & ~7) + 8 :-)
There was a problem hiding this comment.
I used something like this: poolOffset += 8 - poolOffset % 8; :)
There was a problem hiding this comment.
Yeah, I do this to for variable-size alignment or just non-power-of-two.
|
LGTM |
|
@bnoordhuis thanks! @trevnorris : please put your LGTM here too, just to make sure that I'm not messing up with any performance-critical things here. |
|
LGTM |
|
lgtm |
|
oh, last minute LGTM :) I almost pushed the commit without your name in |
When slicing global pool - ensure that the underlying buffer's data ptr is 8-byte alignment to do not ruin expectations of 3rd party C++ addons. NOTE: 0.10 node.js always returned aligned pointers and io.js should do this too for compatibility. PR-URL: #1126 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-by: Bert Belder <bertbelder@gmail.com>
|
Landed in 07c0667, thank you! |
When slicing global pool - ensure that the underlying buffer's data ptr
is 8-byte alignment to do not ruin expectations of 3rd party C++ addons.
NOTE: 0.10 node.js always returned aligned pointers and io.js should do
this to for compatibility.