Skip to content
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.

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

fastly js-compute-runtime patches on top of SM 127.0.2 #44

Merged
merged 16 commits into from
Jul 18, 2024

Conversation

JakeChampion
Copy link
Collaborator

Copy link

(Automated Close) Please do not file pull requests here, see https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html

@github-actions github-actions bot closed this Jul 10, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 10, 2024
@JakeChampion JakeChampion reopened this Jul 10, 2024
elliottt and others added 12 commits July 10, 2024 13:24
Revert "Bug 1795914 - Remove JS Streams implementation r=jandem"
This reverts commit 4852cc8.

Revert "Bug 1807845 - Remove the dom.streams prefs r=evilpie,emilio"
This reverts commit 15d6982.

Revert "Bug 1755391 - Remove JS Streams implementation of Writable Stream and PipeTo r=jandem"
This reverts commit f83d513.

Revert "Bug 1522136 - Remove javascript.options.streams and other legacy prefs. r=mgaudet,webidl,edgar"
This reverts commit c256578.

Revert "Bug 1765060 - Remove dead JS streams code. r=mgaudet"
This reverts commit 75cea93.

Revert "Bug 1774691 - Remove unused JSMSGs. r=mgaudet"
This reverts commit b54b833.

Revert "Bug 1774672 - Remove no longer neaded JS Streams Magic Values r=iain"
This reverts commit d1c4d10.

Revert "Bug 1776013 - Consume mfbt SIMD for String builtins r=iain"
This reverts commit f81ba5c.

Revert "Bug 1779807 - Consume SIMD::memchr64 for array includes/indexof r=iain"
This reverts commit 557de26.

Add the 'js/Stream.h' include back into 'vm/Runtime.h'

Add JSAPI functions for operating on ReadableStream and WritableStream

This adds a few functions for working with ReadableStream, plus introduces a basic set of ones for working with WritableStreams at all.

This patch also includes updates to some of the doc comments for the existing streams API: for many functions the doc comment said that they assert that a passed `Handle<JSObject*>` is (a wrapper for) a specific type, but in reality the function does a check and throws an error instead of asserting. Since this patch introduces a few functions that actually do assert, it seems important to ensure the different behavior is correctly documented.

Fix bug in ReadableStream#pipeTo that causes a failed assert when two actions get queued that trigger a pipeTo shutdown

This can happen e.g. when closing the readable and then erroring the writable end of the pipe in the same microtask.

The Streams spec guards against this by just making Shutdown and ShutdownWithAction bail early if already shutting down, but the implementation was doing an upwrap of the source or destination stream that fails if not guarded by a shuttingDown check because the streams have already been cleared.

Fix bugs in ReadableStream#pipeTo that prevent proper flushing of all pending write requests when the source is closed
Co-authored-by: Jake Champion <[email protected]>
This got lost when fdlibm was re-vendored, because the fix wasn't created as a patch in `modules/fdlibm/patches`. Upstreaming this change will require adding that patch file.
In long-running environments, it's desirable for the embedding to reset the random seed at controlled points in time.

In particular, when running in WASI, a situation can occur where a VM snapshot is taken at build time using [Wizer](https://github.com/bytecodealliance/wizer), which is then loaded and used as the basis for running separate workloads many times over. If `Math.random()` was called during the initialization phase, this would cause all workloads to get the same results for all consecutive calls to `Math.random()`.
…rototype

The flag was previously only respected for internal builtins, not classes defined by the embedding using `JS_InitClass`. That meant one couldn't use `JS_InitClass` to define classes without a globally available constructor.
@JakeChampion JakeChampion force-pushed the fastly/ff-127-0-2 branch 3 times, most recently from 3603111 to ced8341 Compare July 11, 2024 08:02
We have observed some issues that are possibly related to these, and
async resume is still not supported in PBL in any case; let's disable
this for now to see if it addresses immediate issues.
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

LGTM; thanks for working on this!

The "add back DETERMINISTIC_TRACE" commit could be dropped IMHO, unless you'd like to keep it around for further debugging needs; in the absence of that reason I'd prefer to keep the delta between upstream and this in PBL to be zero-ish to make patch-slinging easier :-)

@JakeChampion
Copy link
Collaborator Author

I'll remove the deterministic trace patch during the update to 128 which I will be doing later today hopefully

@JakeChampion JakeChampion marked this pull request as ready for review July 18, 2024 06:20
@JakeChampion JakeChampion merged commit 259ead7 into ff-127-0-2 Jul 18, 2024
8 checks passed
@JakeChampion JakeChampion deleted the fastly/ff-127-0-2 branch July 18, 2024 06:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants