Skip to content

Commit

Permalink
fix: only apply our pipe_to/pipe_through optimisations to TransformSt…
Browse files Browse the repository at this point in the history
…reams who have no transformers.

We have a performance optimisation for when the receiver is a native source, whereby we append the entire source body to the destination using a single native hostcall, this greatly improves the performance (https://barstool.engineering/a-real-world-comparison-between-cloudflare-workers-and-fastly-compute-edge/)

This optimisation however, should only be applied when the TransformStream is being used in this particular, having not supplied a transformer to the TransformStream constuctor. If we apply the optimisation when a transformer is supplied, the transformer would never get called, which is not correct behavior.
  • Loading branch information
JakeChampion committed Jun 9, 2023
1 parent 3ccb4c0 commit e63c8ca
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
22 changes: 21 additions & 1 deletion runtime/js-compute-runtime/builtins/transform-stream.cpp
Expand Up @@ -42,7 +42,15 @@ bool pipeTo(JSContext *cx, unsigned argc, JS::Value *vp) {
JS::RootedObject target(cx, args[0].isObject() ? &args[0].toObject() : nullptr);
if (target && builtins::NativeStreamSource::stream_has_native_source(cx, self) &&
JS::IsWritableStream(target) && builtins::TransformStream::is_ts_writable(cx, target)) {
builtins::NativeStreamSource::set_stream_piped_to_ts_writable(cx, self, target);
auto ts = builtins::TransformStream::ts_from_writable(cx, target);
if (ts) {
auto va = JS::GetReservedSlot(ts, builtins::TransformStream::Slots::HasTransformer);
dump_value(cx, va, stdout);
auto hasGuestTransformer = va.toBoolean();
if (!hasGuestTransformer) {
builtins::NativeStreamSource::set_stream_piped_to_ts_writable(cx, self, target);
}
}
}

return JS::Call(cx, args.thisv(), original_pipeTo, JS::HandleValueArray(args), args.rval());
Expand Down Expand Up @@ -318,6 +326,15 @@ JSObject *TransformStream::writable(JSObject *self) {
return &JS::GetReservedSlot(self, TransformStream::Slots::Writable).toObject();
}

JSObject *TransformStream::ts_from_writable(JSContext *cx, JS::HandleObject writable) {
MOZ_ASSERT(is_ts_writable(cx, writable));
JSObject *sink = builtins::NativeStreamSink::get_stream_sink(cx, writable);
if (!sink || !builtins::NativeStreamSink::is_instance(sink)) {
return nullptr;
}
return builtins::NativeStreamSink::owner(sink);
}

bool TransformStream::is_ts_writable(JSContext *cx, JS::HandleObject writable) {
JSObject *sink = builtins::NativeStreamSink::get_stream_sink(cx, writable);
if (!sink || !builtins::NativeStreamSink::is_instance(sink)) {
Expand Down Expand Up @@ -466,6 +483,9 @@ bool TransformStream::constructor(JSContext *cx, unsigned argc, JS::Value *vp) {
if (!self)
return false;

JS::SetReservedSlot(self, TransformStream::Slots::HasTransformer,
JS::BooleanValue(JS::ToBoolean(transformer)));

args.rval().setObject(*self);
return true;
}
Expand Down
2 changes: 2 additions & 0 deletions runtime/js-compute-runtime/builtins/transform-stream.h
Expand Up @@ -22,6 +22,7 @@ class TransformStream : public BuiltinImpl<TransformStream> {
// used as a body.
UsedAsMixin, // `true` if the TransformStream is used in another transforming
// builtin, such as CompressionStream.
HasTransformer,
Count
};
static const JSFunctionSpec static_methods[];
Expand All @@ -39,6 +40,7 @@ class TransformStream : public BuiltinImpl<TransformStream> {
JS::HandleObject target);
static JSObject *writable(JSObject *self);
static bool is_ts_writable(JSContext *cx, JS::HandleObject writable);
static JSObject *ts_from_writable(JSContext *cx, JS::HandleObject writable);
static JSObject *controller(JSObject *self);
static bool backpressure(JSObject *self);
static JSObject *backpressureChangePromise(JSObject *self);
Expand Down

0 comments on commit e63c8ca

Please sign in to comment.