mirror of
https://github.com/oven-sh/bun.git
synced 2026-10-25 21:40:30 +08:00
Dylan Conwayandautofix-ci[bot]
5e182948be
### What does this PR do?
Moves the stream sinks to bindgen and deletes
`src/codegen/generate-jssink.ts`.
`src/runtime/webcore/Sink.bind.ts` declares four classes:
| Class | What it is |
| --- | --- |
| `ArrayBufferSink` | `Bun.ArrayBufferSink` |
| `FileSink` | `Bun.file(..).writer()`, `Bun.stdout.writer()`,
`proc.stdin` |
| `NetworkSink` | `s3file.writer()` |
| `ReadableSinkController` | the `controller` of a `type: "direct"`
stream that a native sink consumes, and what the pump of any other
stream writes to |
The old generator made a sink class, a constructor and a controller
class for each of the seven sinks. Four of the sink classes were never
instantiated, and the controllers differed only in which sink they
forwarded to. There is now one controller class, as
`ReadableStreamDirectController` in `bun-types` and the docs already
describe it. It holds an enum of typed pointers to the six sinks, and
each operation picks the sink with one `match` in `Sink.rs`.
Bindgen parses the arguments: `write(chunk)` is a union of a string, a
buffer source and anything else, `flush(wait)` a boolean, and
`start(options)` a dictionary of the members that sink reads.
`Bun.file(..).writer(options)` uses the same dictionary as
`FileSink.start`.
The logic that was generated C++ (attach, detach, `end`, `close(error)`,
the closed and destination-closed states) is in `Sink.rs`, and
`BunStreamSource.cpp` keeps the stream side. The two call each other
through member functions of the generated class, which the schema
declares, so neither side declares an `extern "C"` function for them.
This is what `GeneratedSink.h` has for the controller:
```cpp
// Defined in Rust. On exception, a JSValue result is empty.
void close(JSC::JSGlobalObject*, JSC::JSValue error);
JSC::JSValue end(JSC::JSGlobalObject*);
JSC::JSValue write(JSC::JSGlobalObject*, JSC::JSValue data);
bool isDetached();
void closeWithError(JSC::JSGlobalObject*, JSC::JSValue error);
void startWithHighWaterMark(JSC::JSGlobalObject*, double highWaterMark);
JSC::JSValue writeBytes(JSC::JSGlobalObject*, std::span<const uint8_t> bytes);
// Defined in hand-written C++, called from Rust.
JSC::JSValue assignToStream(JSC::JSGlobalObject*, JSC::JSValue stream);
void onClose(JSC::JSGlobalObject*, JSC::JSValue reason, bool sinkClosed);
void onReady(JSC::JSGlobalObject*);
void setPipe(JSC::JSValue stream, JSC::JSValue done);
JSC::JSValue pipeStream();
void clearPipeStream();
void setPipeDone(JSC::JSValue done);
JSC::JSValue pipeDone();
JSC::JSValue takePipeDone();
void setFailReason(JSC::JSValue reason);
JSC::WriteBarrier<JSC::JSCell> m_source;
JSC::WriteBarrier<JSC::JSPromise> m_closePromise;
JSC::WriteBarrier<JSC::Unknown> m_failReason;
JSC::WriteBarrier<WebCore::JSReadableStream> m_readableStream;
JSC::WriteBarrier<WebCore::JSReadableStream> m_pipeStream;
JSC::WriteBarrier<JSC::JSPromise> m_pipeDone;
```
The pump now holds a `JSReadableSinkController*` and calls `write`,
`end` and `close` directly. Before, it looked them up by name and built
an options object for `start`. They are the same functions JS calls, so
arguments convert the same way.
Also gone: `Sink.h`, `generated_jssink.rs`, twelve lazy members of
`ZigGlobalObject`, seven `DOMStructureSlot` entries, six iso subspace
slots, the sink block of `headers.h`, `GlobalObject::assignToStream`,
`host_fn_static` and `StartTag`. `RequestContext` holds its
`HTTPServerWritable` directly, and `JSTransformStream` holds the
controller in one `m_nativeSinkController` in place of a cell, a pointer
and an id.
The sinks were the last Rust code to read a `CallFrame`, so these have
no users and are deleted:
- `CallFrame::{arguments, argument, arguments_count, this}` and the copy
of JSC's register layout that they decoded
- the `CallFrame` parameter of `js_thread_of_caller` and
`context_of_caller`, which leaves one function of each name
- `JSGlobalObject::throw_illegal_constructor`, three `BuiltinName`s,
three methods of `TaggedPtrUnion` and the tag bounds of `TypeList`
- the `flush()` of four sinks, which only the old trait called
#### Bindgen additions
| Option | What it does |
| --- | --- |
| `writable: false` on a class | its native functions are read-only |
| `{ constant: 3 }` in `proto` | a read-only integer, like a WebIDL
`const` |
| `enumerable` on a getter or a constant | overrides the class's, as
`configurable` does |
| `calledFromCpp: true` on a function of `proto` | also a member
function of the generated C++ class: it takes a `JSValue` for each
argument and converts it as a call from JS does |
| `calledFromCpp: { .. }` on a class | member functions defined in Rust
that are not properties; they take and return `JSValue`, `bool`, numbers
and `b.byteSpan` as they are |
| `calledFromRust: { .. }` on a class | member functions that
hand-written C++ defines and Rust calls as `js_Name::x(this_value, ..)`
|
| `b.byteSpan` | bytes that C++ passes to Rust: `std::span<const
uint8_t>`, `&[u8]` |
| `internalFields: { done: b.cell("JSC::JSPromise", header) }` | a field
for hand-written C++, a `WriteBarrier<JSC::JSPromise>`; fields declared
as a list stay `WriteBarrier<Unknown>` |
| `b.string.view` | the characters of a string where they are, as a
`bun_core::StringView<'a>`: no reference is taken and a slice is not
copied |
| `.falsyAsNull` on `b.any` | a falsy value is `None` |
### Behaviour
A script calls every method of an `ArrayBufferSink`, a `FileSink` and a
server's controller with 33 kinds of argument, with no argument, with
five kinds of wrong `this`, and after `close()` and `end()`. It also
covers each member of the `start` and `writer` options, the constructors
and every property descriptor. It ran on main and on this branch; these
are the cases that differ.
**Wrong on main, fixed here**
| Case | main | This PR |
| --- | --- | --- |
| `Bun.ArrayBufferSink.prototype` | `undefined` | the prototype |
| `sink instanceof Bun.ArrayBufferSink` | throws `instanceof called on
an object with an invalid prototype property.` | `true` |
| `class X extends Bun.ArrayBufferSink {}` | throws `The value of the
superclass's prototype property is not an object or null.` | works, and
`Reflect.construct` honours `newTarget` |
**Different, neither wrong**
| Case | main | This PR |
| --- | --- | --- |
| The controller's label in `console.log`, `Object.prototype.toString`
and `heapStats()` | `ReadableHTTPResponseSinkController` and five others
| `ReadableSinkController` |
| `sinkId` of a controller | a data property of each prototype | an
accessor, with the same value and attributes; on the prototype itself it
is `undefined` |
| A sink's method with a wrong `this` | `Expected FileSink`; no `code`
from `close`, `_getFd` and a controller's `end` | `Expected this to be
instanceof FileSink, but received ..`, always with `code:
"ERR_INVALID_THIS"` |
| `FileSink()` without `new` | `Illegal constructor` | `FileSink
constructor cannot be invoked without 'new'` |
| `start(options)` of an `ArrayBufferSink` | `asUint8Array` counts only
if it is an own property | it counts inherited too, like `stream` and
`highWaterMark` |
| `start(options)` of a controller | reads `path` and `fd` for a
`FileSink`, and `highWaterMark` for any other sink | reads all three,
and uses the ones main uses; a getter that throws on one it does not use
makes `start` throw |
| `start({ path, fd })` and `writer({ path, fd })` | `fd` is read only
if `path` is absent | both are read, and `path` still wins |
| `write()` with no argument | `ERR_MISSING_ARGS`, `write() expects a
string, ArrayBufferView, or ArrayBuffer`; a closed sink says that it is
closed, and a controller whose client went away returns 0 |
`ERR_MISSING_ARGS`, `The "chunk" argument must be specified` (`"data"`
on a controller), whatever the state |
| `estimateShallowMemoryUsageOf(new Bun.ArrayBufferSink())` | 96 | 80 |
| `start(options)` with a wrong `this` | runs the getters of `options`,
then throws `ERR_INVALID_THIS` | throws `ERR_INVALID_THIS` first |
| `write(new String(..))` on a closed sink | throws before it converts
the chunk | converts the chunk, then throws the same error |
| A method borrowed across classes | a sink and its controller take each
other's `write`, `flush` and `start`, and a controller of another sink
does not | a sink's methods work on that sink only, and a controller's
on every controller |
**Right on main, wrong here**: none.
`Sink.rs` discarded the result of detaching a controller. It goes to
`dispatch::fold` now, like the same call in `FileSink.rs` and
`streams.rs`, and the `jsresult-swallow` inventory loses the entry.
### Cost
Release builds of main and of this branch, one pinned core, cycles for
each operation. The third column is a second copy of main's binary.
| Operation | main | main again | This PR |
| --- | --- | --- | --- |
| a direct stream to a file | 4594 | 4571 | 4028 |
| `Bun.serve` with a stream body | 12487 | 12537 | 11547 |
| `await Bun.write(path, new Response(stream))` | 8412 | 8439 | 7521 |
| `arrayBufferSink.start({ highWaterMark })` | 243 | 246 | 102 |
| `arrayBufferSink.flush()` | 369 | 369 | 321 |
| `arrayBufferSink.write("hello world")` | 106 | 107 | 89 |
These cost more:
| Call | main | main again | This PR |
| --- | --- | --- | --- |
| `arrayBufferSink.write(uint8Array)` | 76 | 73 | 78 |
| `arrayBufferSink.write(arrayBuffer)` | 90 | 88 | 94 |
| `fileSink.write(uint8Array)` | 101 | 100 | 111 |
| `fileSink.flush()` | 62 | 59 | 69 |
| `controller.write(uint8Array)` | 80 | 82 | 102 |
| `controller.flush()` | 39 | 40 | 51 |
On main `write` and `flush` are each one Rust function that JSC calls
and that reads its own argument. Now a generated C++ function checks
`this` and converts the argument, and then calls the Rust function. A
profile of `controller.write(uint8Array)` shows nothing else: three
frames where main has one, and a call back into C++ for the pointer and
length of the buffer, which `b.bufferSource` reads when the bytes are
used and not when the argument is converted.
The other calls of the three classes (`write` of a string to a
`FileSink` or a controller, `start()`, `end()`, `ref()`, `unref()`,
`_getFd()`, `sinkId`, `new Bun.ArrayBufferSink()`,
`Bun.file(..).writer()`) measure within the difference between the two
copies of main, or faster.
The GC calls Rust when it sweeps a controller. On main the destructor of
a detached controller made no call.
`JsLocal<BufferSource>::with_bytes` gets the bytes from
`Bun__Bindgen__bytesOfBlobOrBuffer`, as a `b.bytes` handle does, in
place of `JSC__JSValue__asArrayBuffer`, which looked at the type again
and filled a struct.
The release binary is 86,016 bytes smaller than main's.
### How did you verify your code works?
- `test/js/bun/util/arraybuffersink.test.ts` and
`test/js/bun/http/serve-direct-readable-stream.test.ts` each have a new
test that fails on main.
- `test/internal/bindgen.test.ts` covers each bindgen addition, and each
mistake in a schema that one of them reports. `TestMemberFunctions` goes
from JS to Rust to C++ (`BindgenTest.cpp`, only in a build with
assertions) and back to Rust, and keeps a `JSPromise` in a typed field
across a GC.
- `test/integration/bun-types/bun-types.test.ts` compares the schema
with `Bun.ArrayBufferSink`, `Bun.FileSink`, `Bun.NetworkSink` and
`ReadableStreamDirectController`. Its list of known differences gains
the members that `bun-types` does not declare (`close`, `_getFd`,
`sinkId`, and `ref` and `unref` of an `ArrayBufferSink`),
`NetworkSink.stat()`, which `bun-types` declares and no sink has, and
the `highWaterMark` of `FileSink.start(options)`, which `bun-types`
declares and a `FileSink` does not read, on main as well.
- These pass on a debug build: `bindgen`, `arraybuffersink`, `filesink`,
`serve-direct-readable-stream`, `direct-readable-stream`,
`async-iterator-stream`, `serve-stream-body-error`,
`s3-upload-stream-gc`, `s3-stream-error-gc`, `spawn-stdin-destroy`,
`spawn-stdin-readable-stream`, `text-encoder-stream`, `compression`,
`body-stream`, `fetch-abort-stream-body`, `html-rewriter`,
`html-rewriter-end-error`, `node-tls-upgrade`.
- `bindgen`, `arraybuffersink`, `filesink`,
`serve-direct-readable-stream`, `serve-stream-body-error`,
`s3-stream-error-gc`, `compression` and `text-encoder-stream` also pass
with `BUN_JSC_validateExceptionChecks=1`.
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>