Add native StringDecoder #1188
Add native StringDecoder #1188
Conversation
github-actions
bot
added
packages:bun
needs tests
| encoding = opt.value(); | ||
| } | ||
| } | ||
| JSStringDecoderPrototype* prototype = JSStringDecoderPrototype::create( |
There was a problem hiding this comment.
We should use LazyClassStructure
If we create a new structure for the prototype each time, the JIT will never optimize this code so it will be slow
I'm assuming more than one instance can be created
| JSStringDecoderPrototype* prototype = JSStringDecoderPrototype::create( | ||
| vm, globalObject, reinterpret_cast<Zig::GlobalObject*>(globalObject)->JSStringDecoderPrototypeStructure()); | ||
| JSStringDecoder* stringDecoder = JSStringDecoder::create( | ||
| vm, globalObject, JSStringDecoder::createStructure(vm, globalObject, prototype), encoding); |
There was a problem hiding this comment.
This structure should be the LazyProperty, not the prototype's structure
| static const HashTableValue JSStringDecoderPrototypeTableValues[] | ||
| = { | ||
| { "write"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsStringDecoderPrototypeFunction_write, 1 } }, | ||
| { "end"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsStringDecoderPrototypeFunction_end, 1 } }, |
There was a problem hiding this comment.
we don't expose "constructor" here?
There was a problem hiding this comment.
looks like it should be LazyClassStructure after all (LazyClassStructure will add constructor and prototype there)
zhuzilin
force-pushed
the
StringDecoder
branch
from
84469d0
to
7d8b2d6
Compare
| } | ||
| int32_t offset = callFrame->uncheckedArgument(1).toInt32(lexicalGlobalObject); | ||
| RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::jsUndefined())); | ||
| if (offset > view->length()) |
There was a problem hiding this comment.
| if (offset > view->length()) | |
| if (offset >= view->length()) |
| { | ||
| return WebCore::subspaceForImpl<JSStringDecoderConstructor, UseCustomHeapCellType::No>( | ||
| vm, | ||
| [](auto& spaces) { return spaces.m_clientSubspaceForStringDecoderConstructor.get(); }, |
There was a problem hiding this comment.
Shouldn't need a subspace for the constructor since no functions/properties are added
| var output = allocator.alloc(u16, len / 2) catch return ZigString.init("Out of memory").toErrorInstance(global); | ||
| var i : usize = 0; | ||
| while (i < len / 2) : (i += 1) { | ||
| output[i] = (@intCast(u16, input[2 * i + 1]) << 8) + @intCast(u16, input[2 * i]); |
There was a problem hiding this comment.
This code is not fast, but I will clean it up
The code is a migration of https://github.com/nodejs/string_decoder. And the tests are from https://github.com/nodejs/node/blob/main/test/parallel/test-string-decoder.js.
Note that I also fixed the
buffer.toString("utf16le").Thank you for your time on this PR :)