Skip to content

Implement Buffer swap16, swap32, swap64 #1659

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.

Sign up for GitHub

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

Merged
Jarred-Sumner merged 2 commits into from
Jan 11, 2023
Merged

Implement Buffer swap16, swap32, swap64 #1659

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98c70f4
Implement Buffer swap16, swap32, swap64
malcolmstill Dec 24, 2022
1b2580e
Initial incorporation of feedback
malcolmstill Dec 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 58 additions & 3 deletions src/bun.js/bindings/JSBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1068,17 +1068,72 @@ static inline JSC::EncodedJSValue jsBufferPrototypeFunction_lastIndexOfBody(JSC:
static inline JSC::EncodedJSValue jsBufferPrototypeFunction_swap16Body(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation<JSBuffer>::ClassParameter castedThis)
{
auto& vm = JSC::getVM(lexicalGlobalObject);
return JSC::JSValue::encode(jsUndefined());
auto scope = DECLARE_THROW_SCOPE(vm);

const int size = 2;
int64_t length = static_cast<int64_t>(castedThis->byteLength());
if (length % size != 0) {
throwRangeError(lexicalGlobalObject, scope, "Invalid buffer length"_s);
return JSC::JSValue::encode(jsUndefined());
}

uint8_t* typedVector = castedThis->typedVector();
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Dec 25, 2022

Choose a reason for hiding this comment

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

Typed Arrays can become "detached". When detached, the vector or typedVector will return a nullptr. That means we have to check for this. It's fortunately pretty uncommon, so we can mark the branch as UNLIKELY

    if (UNLIKELY(castedThis->isDetached())) {
        throwVMTypeError(lexicalGlobalObject, throwScope, "Buffer is detached"_s);
        return JSValue::encode(jsUndefined());
    }


for (size_t i = 0; i < length/size; i++) {
uint8_t temp = typedVector[size*i];
typedVector[size*i] = typedVector[size*i+1];
typedVector[size*i+1] = temp;
}

return JSC::JSValue::encode(castedThis);
}
static inline JSC::EncodedJSValue jsBufferPrototypeFunction_swap32Body(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation<JSBuffer>::ClassParameter castedThis)
{
auto& vm = JSC::getVM(lexicalGlobalObject);
return JSC::JSValue::encode(jsUndefined());
auto scope = DECLARE_THROW_SCOPE(vm);

const int size = 4;
int64_t length = static_cast<int64_t>(castedThis->byteLength());
if (length % size != 0) {
throwRangeError(lexicalGlobalObject, scope, "Invalid buffer length"_s);
return JSC::JSValue::encode(jsUndefined());
}

uint8_t* typedVector = castedThis->typedVector();
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Dec 25, 2022

Choose a reason for hiding this comment

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

need to check for isDetached


for (size_t i = 0; i < length/size; i++) {
for (size_t j = 0; j < size/2; j++) {
uint8_t temp = typedVector[size*i+j];
typedVector[size*i+j] = typedVector[size*i+(size-1)-j];
typedVector[size*i+(size-1)-j] = temp;
}
}

return JSC::JSValue::encode(castedThis);
}
static inline JSC::EncodedJSValue jsBufferPrototypeFunction_swap64Body(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation<JSBuffer>::ClassParameter castedThis)
{
auto& vm = JSC::getVM(lexicalGlobalObject);
return JSC::JSValue::encode(jsUndefined());
auto scope = DECLARE_THROW_SCOPE(vm);

const int size = 8;
int64_t length = static_cast<int64_t>(castedThis->byteLength());
if (length % size != 0) {
throwRangeError(lexicalGlobalObject, scope, "Invalid buffer length"_s);
return JSC::JSValue::encode(jsUndefined());
}

uint8_t* typedVector = castedThis->typedVector();

for (size_t i = 0; i < length/size; i++) {
for (size_t j = 0; j < size/2; j++) {
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Dec 25, 2022

Choose a reason for hiding this comment

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

  1. Ideally, this would use SIMD, leveraging AVX2 on x64 and ARM NEON on aarch64.
  2. We can move size/2 out of the inner loop

uint8_t temp = typedVector[size*i+j];
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Dec 25, 2022

Choose a reason for hiding this comment

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

This is a lot of multiplication. Can we turn this into one add in the outer loop and one add in the inner loop?

typedVector[size*i+j] = typedVector[size*i+(size-1)-j];
typedVector[size*i+(size-1)-j] = temp;
}
}

return JSC::JSValue::encode(castedThis);
}

static inline JSC::EncodedJSValue jsBufferPrototypeFunction_toStringBody(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation<JSBuffer>::ClassParameter castedThis)
Expand Down
78 changes: 78 additions & 0 deletions test/bun.js/buffer.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,84 @@ it("Buffer.from(base64)", () => {
).toBe('console.log("hello world")\n');
});

it("Buffer.swap16", () => {
const examples = [
["", ""],
["a1", "1a"],
["a1b2", "1a2b"]
];
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Dec 25, 2022

Choose a reason for hiding this comment

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

Can we also copy node's tests for this? https://github.com/nodejs/node/blob/master/test/parallel/test-buffer-swap.js#L1-L153


for (let i = 0; i < examples.length; i++) {
const input = examples[i][0];
const output = examples[i][1];
const buf = Buffer.from(input, "utf-8");

const ref = buf.swap16();
expect(ref instanceof Buffer).toBe(true);
expect(buf.toString()).toBe(output);
}

const buf = Buffer.from("123", "utf-8");
try {
buf.swap16();
expect(false).toBe(true);
} catch (exception) {
expect(exception.message).toBe("Invalid buffer length");
}
});

it("Buffer.swap32", () => {
const examples = [
["", ""],
["a1b2", "2b1a"],
["a1b2c3d4", "2b1a4d3c"]
];

for (let i = 0; i < examples.length; i++) {
const input = examples[i][0];
const output = examples[i][1];
const buf = Buffer.from(input, "utf-8");

const ref = buf.swap32();
expect(ref instanceof Buffer).toBe(true);
expect(buf.toString()).toBe(output);
}

const buf = Buffer.from("12345", "utf-8");
try {
buf.swap32();
expect(false).toBe(true);
} catch (exception) {
expect(exception.message).toBe("Invalid buffer length");
}
});

it("Buffer.swap64", () => {
const examples = [
["", ""],
["a1b2c3d4", "4d3c2b1a"],
["a1b2c3d4e5f6g7h8", "4d3c2b1a8h7g6f5e"],
];

for (let i = 0; i < examples.length; i++) {
const input = examples[i][0];
const output = examples[i][1];
const buf = Buffer.from(input, "utf-8");

const ref = buf.swap64();
expect(ref instanceof Buffer).toBe(true);
expect(buf.toString()).toBe(output);
}

const buf = Buffer.from("123456789", "utf-8");
try {
buf.swap64();
expect(false).toBe(true);
} catch (exception) {
expect(exception.message).toBe("Invalid buffer length");
}
});

it("Buffer.toString regessions", () => {
expect(
Buffer.from([65, 0])
Expand Down