buffer list clean-ups (#1721)

1 files changed, 68 insertions, 37 deletions

diff --git a/src/bun.js/bindings/JSBufferList.cpp b/src/bun.js/bindings/JSBufferList.cpp
index d30ec0aa0..e16c8cf88 100644
--- a/src/bun.js/bindings/JSBufferList.cpp
+++ b/src/bun.js/bindings/JSBufferList.cpp
@@ -2,6 +2,7 @@
 #include "JSBuffer.h"
 #include "JavaScriptCore/Lookup.h"
 #include "JavaScriptCore/ObjectConstructor.h"
+#include "JavaScriptCore/JSGenericTypedArrayViewInlines.h"
 #include "ZigGlobalObject.h"
 #include "JSDOMOperation.h"
 #include "headers.h"
@@ -35,22 +36,29 @@ void JSBufferList::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject
 JSC::JSValue JSBufferList::concat(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, int32_t n)
 {
     auto throwScope = DECLARE_THROW_SCOPE(vm);
-    JSC::JSUint8Array* uint8Array = nullptr;
-    if (length() == 0) {
+    auto* subclassStructure = reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure();
+    const size_t len = length();
+    if (len == 0) {
         // Buffer.alloc(0)
-        uint8Array = JSC::JSUint8Array::create(lexicalGlobalObject, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure(), 0);
-
-        RELEASE_AND_RETURN(throwScope, uint8Array);
+        RELEASE_AND_RETURN(throwScope, JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, 0));
+    }
+    auto iter = m_deque.begin();
+    if (len == 1) {
+        auto array = JSC::jsDynamicCast<JSC::JSUint8Array*>(iter->get());
+        if (UNLIKELY(!array)) {
+            return throwTypeError(lexicalGlobalObject, throwScope, "concat can only be called when all buffers are Uint8Array"_s);
+        }
+        RELEASE_AND_RETURN(throwScope, array);
     }
     // Buffer.allocUnsafe(n >>> 0)
     auto arrayBuffer = JSC::ArrayBuffer::tryCreateUninitialized(n, 1);
     if (UNLIKELY(!arrayBuffer)) {
         return throwOutOfMemoryError(lexicalGlobalObject, throwScope);
     }
-    uint8Array = JSC::JSUint8Array::create(lexicalGlobalObject, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure(), WTFMove(arrayBuffer), 0, n);
+    JSC::JSUint8Array* uint8Array = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, WTFMove(arrayBuffer), 0, n);
 
     size_t i = 0;
-    for (auto iter = m_deque.begin(); iter != m_deque.end(); ++iter) {
+    for (const auto end = m_deque.end(); iter != end; ++iter) {
         auto array = JSC::jsDynamicCast<JSC::JSUint8Array*>(iter->get());
         if (UNLIKELY(!array)) {
             return throwTypeError(lexicalGlobalObject, throwScope, "concat can only be called when all buffers are Uint8Array"_s);
@@ -101,29 +109,45 @@ JSC::JSValue JSBufferList::consume(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlob
 JSC::JSValue JSBufferList::_getString(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, int32_t n)
 {
     auto throwScope = DECLARE_THROW_SCOPE(vm);
-    if (n == 0) {
+    if (n == 0 || length() == 0) {
         RELEASE_AND_RETURN(throwScope, JSC::jsEmptyString(vm));
     }
+
+    auto iter = m_deque.begin();
+    JSC::JSString* str = JSC::jsDynamicCast<JSC::JSString*>(iter->get());
+    if (UNLIKELY(!str)) {
+        return throwTypeError(lexicalGlobalObject, throwScope, "_getString can only be called when all buffers are string"_s);
+    }
+    size_t len = str->length();
+
+    if (n == len) {
+        m_deque.removeFirst();
+        RELEASE_AND_RETURN(throwScope, str);
+    }
+    if (n < len) {
+        JSString* firstHalf = JSC::jsSubstring(lexicalGlobalObject, str, 0, n);
+        iter->set(vm, this, JSC::jsSubstring(lexicalGlobalObject, str, n, len - n));
+        RELEASE_AND_RETURN(throwScope, firstHalf);
+    }
+
     JSRopeString::RopeBuilder<RecordOverflow> ropeBuilder(vm);
-    for (auto iter = m_deque.begin(); iter != m_deque.end() && n > 0; ++iter) {
+    for (const auto end = m_deque.end(); iter != end && n > 0; ++iter) {
         JSC::JSString* str = JSC::jsDynamicCast<JSC::JSString*>(iter->get());
         if (UNLIKELY(!str)) {
             return throwTypeError(lexicalGlobalObject, throwScope, "_getString can only be called when all buffers are string"_s);
         }
-        size_t length = str->length();
-        if (length > n) {
+        size_t len = str->length();
+        if (n < len) {
             JSString* firstHalf = JSC::jsSubstring(lexicalGlobalObject, str, 0, n);
             if (!ropeBuilder.append(firstHalf))
                 return throwOutOfMemoryError(lexicalGlobalObject, throwScope);
-
-            JSString* secondHalf = JSC::jsSubstring(lexicalGlobalObject, str, n, length - n);
-            iter->set(vm, this, secondHalf);
+            iter->set(vm, this, JSC::jsSubstring(lexicalGlobalObject, str, n, len - n));
         } else {
             if (!ropeBuilder.append(str))
                 return throwOutOfMemoryError(lexicalGlobalObject, throwScope);
             m_deque.removeFirst();
         }
-        n -= static_cast<int32_t>(length);
+        n -= static_cast<int32_t>(len);
     }
     RELEASE_AND_RETURN(throwScope, ropeBuilder.release());
 }
@@ -131,50 +155,57 @@ JSC::JSValue JSBufferList::_getString(JSC::VM& vm, JSC::JSGlobalObject* lexicalG
 JSC::JSValue JSBufferList::_getBuffer(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, int32_t n)
 {
     auto throwScope = DECLARE_THROW_SCOPE(vm);
-    JSC::JSUint8Array* uint8Array = nullptr;
-    if (n == 0) {
+    auto* subclassStructure = reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure();
+    if (n == 0 || length() == 0) {
         // Buffer.alloc(0)
-        uint8Array = JSC::JSUint8Array::create(lexicalGlobalObject, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure(), 0);
+        RELEASE_AND_RETURN(throwScope, JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, 0));
+    }
 
-        RELEASE_AND_RETURN(throwScope, uint8Array);
+    auto iter = m_deque.begin();
+    JSC::JSUint8Array* array = JSC::jsDynamicCast<JSC::JSUint8Array*>(iter->get());
+    if (UNLIKELY(!array)) {
+        return throwTypeError(lexicalGlobalObject, throwScope, "_getBuffer can only be called when all buffers are Uint8Array"_s);
+    }
+    size_t len = array->byteLength();
+
+    if (n == len) {
+        RELEASE_AND_RETURN(throwScope, array);
+    }
+    if (n < len) {
+        auto buffer = array->existingBuffer();
+        JSC::JSUint8Array* retArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, 0, n);
+        JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, buffer, n, len - n);
+        iter->set(vm, this, newArray);
+        RELEASE_AND_RETURN(throwScope, retArray);
     }
+
     // Buffer.allocUnsafe(n >>> 0)
     auto arrayBuffer = JSC::ArrayBuffer::tryCreateUninitialized(n, 1);
     if (UNLIKELY(!arrayBuffer)) {
         return throwTypeError(lexicalGlobalObject, throwScope);
     }
-    uint8Array = JSC::JSUint8Array::create(lexicalGlobalObject, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure(), WTFMove(arrayBuffer), 0, n);
-
+    JSC::JSUint8Array* uint8Array = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, WTFMove(arrayBuffer), 0, n);
     size_t offset = 0;
-    for (auto iter = m_deque.begin(); iter != m_deque.end() && n > 0; ++iter) {
+    for (const auto end = m_deque.end(); iter != end && n > 0; ++iter) {
         JSC::JSUint8Array* array = JSC::jsDynamicCast<JSC::JSUint8Array*>(iter->get());
         if (UNLIKELY(!array)) {
             return throwTypeError(lexicalGlobalObject, throwScope, "_getBuffer can only be called when all buffers are Uint8Array"_s);
         }
-        size_t length = array->byteLength();
-        if (length > n) {
+        size_t len = array->byteLength();
+        if (n < len) {
             if (UNLIKELY(!uint8Array->setFromTypedArray(lexicalGlobalObject, offset, array, 0, n, JSC::CopyType::Unobservable))) {
                 return throwOutOfMemoryError(lexicalGlobalObject, throwScope);
             }
-            // create a new array of size length - n.
-            // is there a faster way to do this?
-            auto arrayBuffer = JSC::ArrayBuffer::tryCreateUninitialized(length - n, 1);
-            if (UNLIKELY(!arrayBuffer)) {
-                return throwOutOfMemoryError(lexicalGlobalObject, throwScope);
-            }
-            JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(
-                lexicalGlobalObject, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject)->JSBufferSubclassStructure(), WTFMove(arrayBuffer), 0, length - n);
-
-            memcpy(newArray->typedVector(), array->typedVector() + n, length - n);
+            JSC::JSUint8Array* newArray = JSC::JSUint8Array::create(lexicalGlobalObject, subclassStructure, array->existingBuffer(), n, len - n);
             iter->set(vm, this, newArray);
         } else {
-            if (UNLIKELY(!uint8Array->setFromTypedArray(lexicalGlobalObject, offset, array, 0, length, JSC::CopyType::Unobservable))) {
+            if (UNLIKELY(!uint8Array->setFromTypedArray(lexicalGlobalObject, offset, array, 0, len, JSC::CopyType::Unobservable))) {
                 return throwOutOfMemoryError(lexicalGlobalObject, throwScope);
             }
             m_deque.removeFirst();
         }
-        n -= static_cast<int32_t>(length);
-        offset += length;
+        n -= static_cast<int32_t>(len);
+        offset += len;
     }
     RELEASE_AND_RETURN(throwScope, uint8Array);
 }