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

use native .reverse if available #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rubensayshi
Copy link

No description provided.

@dcousens
Copy link
Member

can you safe-buffer too?

@rubensayshi
Copy link
Author

idk why but tests fail ... src.reverse() is not reversing it o.O

@rubensayshi
Copy link
Author

rubensayshi commented Jul 31, 2017

// wtf.js
var a = new Buffer("00ff", 'hex')
console.log(a, a.reverse(), a)

var b = new Buffer("ff00", 'hex')
console.log(b, b.reverse(), b)

var c = new Buffer("00ff", 'hex')
console.log(c)

var d = c.slice()
console.log(d, d.reverse())
<Buffer ff 00> <Buffer ff 00> <Buffer ff 00>
<Buffer 00 ff> <Buffer 00 ff> <Buffer 00 ff>
<Buffer 00 ff>
<Buffer ff 00> <Buffer ff 00>

wtf is going on here o.O

@rubensayshi
Copy link
Author

// wtf2.js
var a = new Buffer("00ff", 'hex')
console.log(a)
console.log(a.reverse())
console.log(a)

var b = new Buffer("ff00", 'hex')
console.log(b)
console.log(b.reverse())
console.log(b)

var c = new Buffer("00ff", 'hex')
console.log(c)

var d = c.slice()
console.log(d)
console.log(d.reverse())
<Buffer 00 ff>
<Buffer ff 00>
<Buffer ff 00>
<Buffer ff 00>
<Buffer 00 ff>
<Buffer 00 ff>
<Buffer 00 ff>
<Buffer 00 ff>
<Buffer ff 00>

@rubensayshi
Copy link
Author

oh it reverses in-place and when I console.log them together they all the same xD

@fanatid
Copy link
Contributor

fanatid commented Jul 31, 2017

yea, moreover slice refer to original buffer, the real copy will be: Buffer.from

var a = new Buffer("00ff", 'hex')
console.log(a, Buffer.from(a).reverse())
// <Buffer 00 ff> <Buffer ff 00>
console.log(a, a.slice().reverse())
// <Buffer ff 00> <Buffer ff 00>

@rubensayshi
Copy link
Author

ugh made a mess out of the codestyle ...

@rubensayshi
Copy link
Author

there, with safe-buffer / Buffer.from it passes :D

@dcousens
Copy link
Member

dcousens commented Jul 31, 2017

slice will be [often] a copy in browser, and a view in node.

reverse is in-place everywhere 👍

index.js Outdated
if (typeof src.reverse === "function") {
return Buffer.from(src).reverse()
} else {
var buffer = Buffer.alloc(src.length)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allocUnsafe would be fine too

@dcousens
Copy link
Member

dcousens commented Oct 2, 2018

@calvinmetcalf this had safe-buffer too ha... woops.
Sorry @rubensayshi

@calvinmetcalf
Copy link
Contributor

haha woops

@calvinmetcalf
Copy link
Contributor

can you try rebasing on top of master? if you have trouble with that I can do it for you

@dcousens
Copy link
Member

dcousens commented Oct 2, 2018

@rubensayshi when using Buffer.from(buf).reverse(), there was always a 100% performance penalty... (2x slower) I don't think this is worth it.

For the inplace case, it is about 30% faster.

@calvinmetcalf
Copy link
Contributor

so it sounds like we just want this for the inplace one, which nicely wouldn't conflict with the safebuffer stuff

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

Successfully merging this pull request may close these issues.

4 participants