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

Merge facebook/hermes version RN 0.74.3 #196

Merged
merged 761 commits into from
Aug 13, 2024
Merged

Conversation

jonthysell
Copy link

@jonthysell jonthysell commented Aug 9, 2024

Summary

The purpose of this PR is to upgrade hermes-windows with the latest code from upstream, specifically the hermes-2024-06-28-RNv0.74.3-7bda0c267e76d11b68a585f84cfdd65000babf85.

Test Plan

Unit tests pass

Microsoft Reviewers: Open in CodeFlow

neildhar and others added 30 commits November 30, 2023 08:58
Summary:
X-link: facebook/react-native#41436

Add a JSI API for associating some native memory with a JS object. This
is intended to provide a mechanism to trigger more frequent garbage
collection when JS retains large external memory allocations, in order
to avoid memory buildup.

This diff just adds the JSI method, without any implementations.

Changelog:
[General][Added] - Added JSI method for reporting native memory to the GC.

Reviewed By: tmikov

Differential Revision: D50524912

fbshipit-source-id: c8df0e18b0415d9523e0a00f6d0ed2faa648ac68
Summary: Pull Request resolved: facebook/react-native#41725

Reviewed By: SamChou19815

Differential Revision: D51713020

fbshipit-source-id: 84dc8c882f7603d642cb4ccb735fad0fb7e25d4d
Summary:
Pass the GC and `NativeState*` to the finalizer to make it more
flexible. This will allow it to be used to manage external memory in
the next diff.

Reviewed By: tmikov

Differential Revision: D50531361

fbshipit-source-id: f5483f0e25d92156bc4f593ffa0f19c6de88a1c0
Summary:
It turned out that these assertions hit with test262 tests. These test
cases are, for example, passing property descriptor with incomplete
attributes. For some tests, this doesn't matter and they are just
testing the behavior of trap functions. Few test cases are testing with
incorrect attribute set up and checking that the runtime is handling as
spec says. Essentially, `objectFromPropertyDescriptor` should create the
descriptor object as specified by the code, and Proxy should pass it to
the `defineProperty` trap function as is. Each of the case handling is
already well  implemented in the Proxy code.

Reviewed By: tmikov

Differential Revision: D51692787

fbshipit-source-id: 340b49007ca7a35eacab07be1c1e40a6e5b7ec7d
Summary:
Add an implementation of `adjustExternalMemoryPressure` in Hermes. It
uses an internal property pointing to a `NativeState` to maintain the
external memory size. The finalizer of the `NativeState` informs the GC
that the memory has been freed.

Reviewed By: tmikov

Differential Revision: D51241265

fbshipit-source-id: d9be03ee71d34a75fc6a2544e34ca98d08dfa70d
Summary:
This change eliminates repeated calls to `ucol_open` and `ucol_close` in `hermes::platform_unicode::localeCompare` for HERMES_PLATFORM_UNICODE_ICU. On a sample sort.js(that sorted 30k strings using localeCompare), the runtime goes down from ~500 ms to ~120ms.

**Before:**
I observed in my trace that ucol_open and ucol_close was called in every localeCompare invocation and each ucol_open took around 500-900 ns. ucol_close is tiny ~100ish ns in each call.
Note that the first call to ucol_open, it takes around 100us even after applying this diff.
![image](https://github.com/facebook/hermes/assets/6753926/739edffb-6c7b-4e93-babd-f51f9add4231)

Here is localeCompare call stack before the patch:
![image](https://github.com/facebook/hermes/assets/6753926/9b9fbf31-aa5f-4cc6-a594-26c544c6ebcb)

**After**
After making UCollator construction static, we avoid repeated ucol_open and ucol_close(except during static initialization)  thus making the sort.js sorting fast.
Here is the localeCompare stack after the patch:
![image](https://github.com/facebook/hermes/assets/6753926/7deeba56-ec64-480d-9cf2-4ddcec10fc8b)

Looking at the code, i found out that HERMES_PLATFORM_UNICODE_ICU is not used in Android/iOS but can make desktop usage faster (it it useful??).

Here is the sort.js script which i got from one of the issues/posts in hermes.
```
print("gen...");
  const randInt = (end, start = 0) =>
    Math.floor(Math.random() * (end - start) + start);
  const alphabet = 'abcdefghijklmnopqrstuvwxyz';
  const names = Array.from({length: 30000}, () =>
     Array.from({length: 8}, () => alphabet[randInt(alphabet.length)]).join(''),
   );

  for(var i = 0; i < 5; ++i){
    print(names[i])
  }
  print('sorting...');
  const s = Date.now();
  names.sort(localeCompare);
  print(`...done, took ${Date.now() - s}ms`);

  function localeCompare(a, b) {
    const s = Date.now();
    const result = a.localeCompare(b);
    const e = Date.now();
    if (e - s > 1) {
      print(`slow localeCompare: ${e - s}ms for '${a}' vs '${b}'`);
    }
    return result;
  }

  for(var i = 0; i < 5; ++i){
    print(names[i])
  }
```

Pull Request resolved: facebook#1186

Test Plan:
**Execute sort.js before applying this patch**
```
hermes (main)$ ../build_release/bin/hermesc -emit-binary -out sort.hbc samples/sort.js
hermes (main)$ ../build_release/bin/hvm ./sort.hbc
gen...
sorting...
...done, took 466ms
hermes (main)$ ../build_release/bin/hvm ./sort.hbc
gen...
sorting...
...done, took 486ms
```

**Execute sort.js after applying this patch**
```
$ ../build_release/bin/hermesc -emit-binary -out sort.hbc samples/sort.js
hermes (icu_localecompare_optimize)$ ../build_release/bin/hvm ./sort.hbc
gen...
sorting...
...done, took 119ms
hermes (icu_localecompare_optimize)$ ../build_release/bin/hvm ./sort.hbc
gen...
sorting...
...done, took 127ms
```

**Run check-hermes:**
```
hermes (icu_localecompare_optimize)$ cmake --build ../build_release/ --target check-hermes
[0/1] Running the Hermes regression tests
Testing Time: 15.91s
  Expected Passes    : 1773
  Unsupported Tests  : 66
hermes (icu_localecompare_optimize)$ echo $?
0
```

Reviewed By: avp

Differential Revision: D51429400

Pulled By: neildhar

fbshipit-source-id: f5a2f56952ec9bcbaad441df8f2e9c49c8a0b09a
…cebook#1202)

Summary:
Pull Request resolved: facebook#1202

Relational comparisons (<= and >=) are simplified to a true/false when
it is known that the operands are the same value with a primitive type.
If the value is "undefined", it is converted to NaN, which never equals
itself.
The optimization was not correctly handling the case where the operand
could be "undefined" or another type - it was only checking for
"undefined".

Example:

    try {} catch {
      var v0 = "m";
    }
    return v0 <= v0;

The try/catch is necessary to make the optimizer believe `v0` is not a
constant.

Reported in facebook#1200.

Reviewed By: neildhar

Differential Revision: D51555324

fbshipit-source-id: 87a4285166a9e11e96014feae45497d1a30d8307
Summary: Changelog: [Internal]

Reviewed By: pieterv

Differential Revision: D51768863

fbshipit-source-id: 716add02d82cdfa56f2d3fc4d4560fcb26e8f426
Summary:
Hermes parser currently cannot handle TS files as good as Flow ones. However, the plugin will unconditionally override the parser for all files. This will break if we introduce the plugin in a config that must work for both TS and JS.

This diff skips the parser override if the filename ends in .ts or .tsx. According to babel's implementation, we need to return undefined to ignore the parserOverride:

https://github.com/babel/babel/blob/85e649203b61b7c908eb04c05511a0d35f893e8e/packages/babel-core/src/parser/index.ts#L9-L25

Reviewed By: pieterv

Differential Revision: D51771100

fbshipit-source-id: 7564856952c02d5707536c68a835603aaee77de2
Summary:
`getBinarySideEffect()` wasn't considering that arithmetic operations
between BigInt and non-BigInt (except string addition) throw a
TypeError.

Refactor the code to be explicit about all operators. Add new tests.

Curiously, this change revealed a bug in an existing test:

        t5: string|number|bigint = t1:any + t2:any
        t6: string|number|bigint = t3:any + t4:any
        t7: string|number|bigint = t5 + t6
        t7 is unused

We were incorrectly removing the calculation of t7, because both
operands are primitive types. However it might happen, for example, that
t5 is biging and t6 is a number, which would throw an exception.

Reported in facebook#1199.

This diff was ported from Static Hermes (since this is a bug). Had to
fix BigInt dumping to make the Optimizer test pass.

Reviewed By: avp

Differential Revision: D51555323

fbshipit-source-id: 927f5e6041899f0800c5a641c62d32e18c3d9f18
Summary:
If an instruction is the last user of one of its operands, the register
allocator may reuse one of its operands as the result register. This is
typically fine since most operands are only input parameters, and they
are always read before the output is written.

However, when the operand is both an input and an output, this requires
care to ensure that the operand register (whose live range ends with
the given instruction) is written before the result. This ensures that
even if the register allocator assigns them the same register, the
value held in that register after the instruction is the result.

Closes facebook#1203

Reviewed By: avp

Differential Revision: D51597086

fbshipit-source-id: 14a4f27887d11cbe21bae5b27712b498b48c3f20
Summary:
In debug mode, this assertion is used to catch potential unbounded
handle allocations. It offers however very poor signal when fuzzing.
Skipping it to unblock stress-testing large GCScopes.

Reviewed By: neildhar

Differential Revision: D51587303

fbshipit-source-id: 5d373ecee8535bf38ba8f56055ff9efa03cadec3
Summary:
Newer versions of clang will automatically produce FMA instead of
separate multiply and add when the multiplication and addition are in
the same expression. This is undesirable because it results in
non-standard behaviour in makeDate, and breaks an existing test262
test.

We can do this in two ways:
1. We can split them into separate expressions
2. We can use `#pragma STDC FP_CONTRACT OFF`

Unfortunately, neither of these address the issue with GCC, which
uses `-ffp-contract=fast` by default, and cannot be overridden
except by using `volatile`.

To fix this, add `-ffp-contract=on` to our build flags, and split the
expressions.

Reviewed By: tmikov

Differential Revision: D51725454

fbshipit-source-id: 3d4c91bb32c619c05c53fd8b1962adbc862554aa
Summary:
X-link: facebook/react-native#41797

Changelog: [Internal]

Reviewed By: gkz

Differential Revision: D51839509

fbshipit-source-id: 961fc4e8fc0f7a45c3882753301d027f319b9ea6
Differential Revision:
D51839509

Original commit changeset: 961fc4e8fc0f

Original Phabricator Diff: D51839509

fbshipit-source-id: 71a97ee1971d5c882cb704f4f9ff42a95e6d52f6
Summary:
X-link: facebook/react-native#41809

Changelog: [Internal]

Reviewed By: samwgoldman

Differential Revision: D51855821

fbshipit-source-id: 4fe9c937aff925150fb015be2ea93fa120e7f462
Summary:
We are currently experimenting with a Hermes integration as a potential replacement for the QuickJS engine. As part of the migration, we've realized that the lack of support for ES6 classes and block scoped variables within Hermes is a bigger deal than expected for us. We have 3 engine integrations which all support ES2017+, and without support for ES6 classes and block scoped variables we have to either downgrade the ES target to ES5, or integrate Babel in our compilation pipeline which are both invasive and risky changes for us. In order to fix this, I've taken a stab at supporting ES6 classes somewhat natively within Hermes.

I've went with the simplest and the less risky approach that could get the job done. This PR doesn't change the IR, it instead transforms the AST at compile time to convert ES6 classes into plain ES5 classes, very much like what the TypeScript and Babel compiler do. A more advanced implementation would likely add support for classes within the IR. This PR ended up to be fairly straightforward to do, as the JS parser implemented within Hermes already supports most of the ES6 syntax, it's only the IR Gen pass which doesn't support it.

## Supported ES6 class features
- Define a class with a parent class (`class Child extends Parent {}`)
- Method and static method definitions (`method() {}` and `static method() {}`)
- Property and static property definitions (`get prop() {}`, `static get prop() {}`, `set prop(v) {}`, `static set prop(p) {}`
- Super call expression in method and static method definitions (`super.method()`)
- Super member expression in property and static property definitions (`super.property`)
- User defined constructor (`constructor() {}`)
- Properties that are immediately initialized. (`property = someValue`)
- Class expressions (`const MyClass = class {}`)
- Class expressions with private named identifier (`const MyClass = class Self { constructor() { Self.hello = 'world' }})`)

## Unsupported ES6 class features
- static block initializers (`static { // code to eval at class load time }`). The existing parser is not able to parse these blocks yet.
- Private methods and properties (`#privateMethod() {}`)
- Inheritance from built-in types like `Error` or `Map` (`class CustomError extends Error {}`)

## Changes

- Added a `enableES6Classes` compile flag. When enabled, a new post processing step will occur on the AST which will transform ES6 classes into ES5 functions, similar to what Babel or the TypeScript compiler does.
- Added `lib/AST/ES6Class`, an AST visitor which visits the ClassNodeDeclaration and ClassExpressionNode and will turn them into plain ES5 functions. The generated AST relies on a new helper lib that is meant to be available on the global object as `HermesES6Internal`.
- Added the `HermesES6Internal` helper module, which implements `defineClass`, `defineClassProperty`, ` defineStaticClassProperty`, `defineClassMethod` and `defineStaticClassMethod`.
- Added a few tests

Pull Request resolved: facebook#1176

Test Plan: - All Hermes tests are passing

Reviewed By: tmikov

Differential Revision: D51713522

Pulled By: avp

fbshipit-source-id: 15e5f3cd0762f7ffddb357f1ec9edce9713bbafe
Summary:
the StringLiteral comparison here doesn't actually work. also NumericLiterals are valid as property keys, so it adds that as well

this change effects object and class properties and methods.

Reviewed By: pieterv

Differential Revision: D51867940

fbshipit-source-id: ad546b7c539d29c80fc83393c2b5b66678d51b43
Summary:
The initial boilerplate for the WIP Hermes stable ABI. This just
creates a directory with the header defining the new interface, which
will be used by both the wrapper and the actual implementation of the
ABI.

Reviewed By: tmikov

Differential Revision: D47563699

fbshipit-source-id: b6da43b3f8fdd168d8bda3adee53277d17275325
Summary:
Create a skeleton for the concrete implementation of our new C-API on
top of the Hermes internal APIs. Set up a build that will build either
a static or dynamic library depending on the setting of
`BUILD_SHARED_LIBS`.

Reviewed By: dannysu

Differential Revision: D50153971

fbshipit-source-id: e8462f914a076efc575d6ad9cec13cc9c4627e09
Summary:
Add the boilerplate for the C++ wrapper that implements JSI on top of
the C-API. Currently, each function is just implemented to throw.

Reviewed By: tmikov, avp

Differential Revision: D50153972

fbshipit-source-id: bea12505e75549c13a281fe9153875471ccd6ee5
Summary:
It turned out that these maps are holding the HostObjects and
HostFunctions created during replay, and never be released until the
runtime instance is destroyed. This changes the behavior of GC for the
host object during replay compared to tracing time as there objects can
never be collected. These maps are used only for assertions to make sure
the created IDs are unique, but they are created by the real runtime's
function, so such assumptions should be able to be made already. Hence,
simply removing these members and usages.

Reviewed By: neildhar

Differential Revision: D51874351

fbshipit-source-id: 1ca93c35b4abbc78b95e0a24419f820e16946fac
Summary: Additional Fuzzilli profile for sandbox memory fuzzing.

Differential Revision: D51352212

fbshipit-source-id: 106a4c89704284c0c6c3f99e96f52869f225e3dc
Summary:
We've added `jsi::Runtime::setExternalMemoryPressure()`.
Let's add tracing and replaying for SynthTrace.

Note that function call to `setExternalMemoryPressure()`
won't affect correctness of the inputs and outputs during
replay, but we want to replay for performance effects
mostly for GC.

Reviewed By: neildhar

Differential Revision: D51835433

fbshipit-source-id: 0701bc2d83fe0f724ed3a215ef53ff016b9f5a58
Summary: Capitalise the macro and fix the include guard.

Reviewed By: tmikov, ftanuma

Differential Revision: D51923607

fbshipit-source-id: f7f978fb04ee566af2ffce05d29b04a1c1ef2945
Summary:
CDPHandler needs to know the list of loaded scripts so that when `Debugger.enable` message comes, CDPHandler can emit `Debugger.scriptParsed` notifications. Currently CDPHandler obtains the loaded scripts by requiring that it be instantiated along with HermesRuntime, so that CDPHandler can monitor `PauseReason::ScriptLoaded` and cache the list of scripts.

However, this is not a desirable design because it introduces the unnecessary requirement that CDPHandler must be created along with HermesRuntime even though there is no CDP connection. It introduces the question of "what does the msgCallback do when there is no connection?"

Further examination revealed that there is actually no need for CDPHandler to be doing this caching. This is a sensible API to expose from DebuggerAPI. By exposing this functionality, we also save memory spent on the cache by not duplicating information already exists in the VM.

I also made a change to align CMake with BUCK. The CMake build emits debug info for InternalBytecode whereas the BUCK build doesn't. `hermes_compile` by default builds with `-g0`. When debug info is present, iterating through the RuntimeModules would get InternalBytecode.js included as one of the loaded scripts. That by itself isn't a problem. I just thought it'd be good to be building things the same way.

Reviewed By: mattbfb

Differential Revision: D51684983

fbshipit-source-id: b838f71674320bd641ce47f43d7bff14a1312ff5
Summary:
I noticed that we don't have test case nor trace record that exercises
`HostObject`'s `setProperty`` calls. Adding a test case revealed that
thereplay part isn't working, so fixing it here.

Here are short description of bugs
- `FakeHostObject::set()` was not passing
  `PropNameID *nativePropNameToConsumeAsDef` to
  `interpreter.execFunction`, which was actually needed.
- `RecordType::GetPropertyNativeReturn` case was using `getObjForUse`
   when calling `traceValueToJSIValue` even though value can be
   String, BigInt, etc (non Object). `getObjForUse()` internally calls
   `jsi::Value::getObject()` without type check so it asserts in debug
   builds.
- `RecordType::SetPropertyNativeReturn` was doing unnecessary
   operations copied from `RecordType::SetPropertyNative`.

Reviewed By: neildhar

Differential Revision: D51966162

fbshipit-source-id: 5f4cec2b4db4bd125b36e06ae1d5ec40000b05da
Summary:
There is no good reason to have HostObject's call count as
TraceInterpreter's member. We can have it in each FakeHostObject's
instance.

Reviewed By: neildhar

Differential Revision: D51920392

fbshipit-source-id: 50573ae5142efcba90b7c35467cbd268820fa0d4
Summary:
Previous Diff broke the builds on CircleCI.
It was missing include for variant.

Pull Request resolved: facebook#1217

Reviewed By: neildhar

Differential Revision: D52045636

fbshipit-source-id: 930f1b3af05b3a80281f6050d36489390a939391
…nterpreter

Summary:
There is no good reason to have HostObject's getPropertyNames() call
count as TraceInterpreter's member. We can have it in each
FakeHostObject's instance.

Reviewed By: neildhar

Differential Revision: D51966363

fbshipit-source-id: fd4155c4c15b18ce94d38570f28d1c614d2835d1
dannysu and others added 18 commits February 16, 2024 21:20
Summary:
Changing our buck build to be done in a similar way as our cmake build.
The HermesAPI target is updated to include `RemoteObjectsTable.cpp`.
This way integrators just need to include HermesAPI (or the lean
version), just like how cmake build works. I'm doing
`RemoteObjectsTable.cpp` first to prove out the idea. If this is good,
then I'll do the new CDPAgent the same way.

I also had to move `RemoteObjectsTable.cpp` from the `inspector/chrome`
directory to avoid buck warning that it should belong to a target
inside `inspector/chrome` because there's a BUCK file there.

Reviewed By: mattbfb

Differential Revision: D53839508

fbshipit-source-id: abf2813fc545d9f2eac05a8982466c20578e8a12
Summary:
This diff adds just the TextEncoder class and constructor. Other
functions are added with subsequent diffs.

Reviewed By: fbmal7

Differential Revision: D53212825

fbshipit-source-id: c38c1c8cee880c49f62ced238d7f2a2a3650d941
Summary: Adds the read-only `encoding` property to TextEncoder

Reviewed By: avp

Differential Revision: D53212867

fbshipit-source-id: 010875ff359aaaa8e84a185c8d716bc4689a1538
Summary:
The ECMAScript spec has a function that does exactly the same thing, so
renaming `decodeSurrogatePair` to indicate that it's spec compliant.

Reviewed By: avp

Differential Revision: D53456339

fbshipit-source-id: 293822f5f7bad832cc2b7a57941ef67872320ebb
Summary:
The `convertUTF16ToUTF8WithReplacements()` is actually a spec compliant
implementation of the UTF-8 Encoder:
https://encoding.spec.whatwg.org/#utf-8-encoder

Added comments to clarify what we pass to `encodeUTF8()` is always a
scalar value.

Reviewed By: avp

Differential Revision: D53456530

fbshipit-source-id: 1160b0907523c0a53c07ee825c9124ec16f6f831
Summary: Implement TextEncoder's `encode()` function.

Reviewed By: avp

Differential Revision: D53213625

fbshipit-source-id: 706d57c9bd155872e07b2547028db7bdb045609f
Summary: Implement TextEncoder's `encodeInto()` function.

Reviewed By: avp

Differential Revision: D53216139

fbshipit-source-id: eb4f5a1461084d22c77a7c0723de624f50468785
Summary:
Changelog: [Internal]

Hermes:

Adds the missing `validateExecutionContext` call to `Runtime.evaluate`.

React Native:

Adds an integration test case to cover the expected behaviour around targeting `Runtime.evaluate` by execution context.

bypass-github-export-checks

Reviewed By: huntie

Differential Revision: D53776532

fbshipit-source-id: 66676383ba5b373fdbf2deb8c75f22791b07e300
)

Summary:
According to the API documentation [GetProcessMemoryInfo function (psapi.h)
](https://learn.microsoft.com/en-us/windows/win32/api/psapi/nf-psapi-getprocessmemoryinfo)

> If the function succeeds, the return value is nonzero.
> If the function fails, the return value is zero. To get extended error information, call [GetLastError](https://learn.microsoft.com/en-us/windows/desktop/api/errhandlingapi/nf-errhandlingapi-getlasterror).

Pull Request resolved: facebook#1315

Test Plan:
- Turn on ShouldRecordStats ( GCConfig.h )
- Be able to get the actual value instead of 0

Reviewed By: neildhar

Differential Revision: D53901267

Pulled By: tmikov

fbshipit-source-id: 9053e0e94eddee30a9bcf6b50e32bd01a0172eb5
Summary:
X-link: facebook/react-native#43098

Changelog: [Internal]

Wraps Hermes's `CDPHandler::getState()` API in an engine-agnostic abstraction (`RuntimeAgentDelegate::getExportedState`).

An Agent's lifetime ends when its Target is destroyed, but it can occasionally be useful to persist some state for the "next" Target+Agent of the same type (in the same session) to read.

`RuntimeAgentDelegate` is polymorphic and can't just write arbitrary data to SessionState. Instead, it can now *export* a state object that we'll store and pass to the next `RuntimeTargetDelegate::createAgentDelegate` call.

Reviewed By: huntie

Differential Revision: D53919696

fbshipit-source-id: a8e9b921bc8fc2d195c5dddea9537e6ead3d0358
Summary: Backports D56031659 from `CDPAgent` to `CDPHandler` in order to fix a visible regression from RN 0.72 --> 0.73 (which introduced the breakage in `CDPHandler`).

Reviewed By: mattbfb

Differential Revision: D56417968

fbshipit-source-id: 04e9ae17c83b1acf1960767efb81aa8b1ac34345
Summary:
The forgiving-base64 decode algorithm
(https://infra.spec.whatwg.org/#forgiving-base64) gives specific
instructions for if remainder is 0 and if remainder is 1.

The only error case is when the remainder is 1:
> If data’s code point length divides by 4 leaving a remainder of 1,
then return failure.

When the remainder is 2 or 3, it should still be considered valid
base64 data.

Fixing `base64DecodeOutputLength()` so we're not returning error
incorrectly.

Reviewed By: avp

Differential Revision: D56480630

fbshipit-source-id: ef500ddd6ed1a128150a79f6d7d8ae4ef8be4f2d
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536

fbshipit-source-id: 25f52f91d7ef1a51687c431d2c7562c373dc72a5
Summary: This adds a basic implementation for `jsi::Runtime::queueMicrotask` in Hermes that adds the callbacks to the existing microtask queue in Hermes (used in `drainMicrotasks`).

Reviewed By: neildhar

Differential Revision: D54302535

fbshipit-source-id: f62b83b972f8610699380368e15b0db253ba2324
Summary:
X-link: facebook/react-native#43311

Pull Request resolved: facebook#1337

Changelog: [internal]

We've done this in a separate diff because the changes in Hermes don't propagate immediately to the React Native repository. We need to land the changes in JSI and Hermes first (in a backwards-compatible way) and then land this in a separate commit to make the method mandatory.

Reviewed By: neildhar

Differential Revision: D54413830

fbshipit-source-id: 3b89fe0e6697b0019544b73daa89d932db97b63a
doc/IntlAPIs.md Outdated Show resolved Hide resolved
website/yarn.lock Outdated Show resolved Hide resolved
This commit is to clean-up issues introduced during the merge to `hermes-2024-06-28-RNv0.74.3-7bda0c267e76d11b68a585f84cfdd65000babf85`.
@jonthysell jonthysell marked this pull request as ready for review August 12, 2024 17:14
@jonthysell jonthysell requested a review from a team as a code owner August 12, 2024 17:14
@jonthysell
Copy link
Author

@vmoroz I'm only seeing the option to squash and merge these 761 commits from upstream. But that means hermes-windows will continue to be over 1000 commits behind upstream, and we never have those commits in our code history.

Would it be possible to enable merge commits for integrates? Or perhaps even better, reset main to match/track upstream, and then create and publish our actual build a from our own different windows branch? (We can even make the windows branch the default).

@vmoroz vmoroz merged commit 4c64b05 into microsoft:main Aug 13, 2024
10 checks passed
vmoroz added a commit that referenced this pull request Aug 13, 2024
Merge remote-tracking branch 'upstream/main' into rnw/abi
@jonthysell jonthysell deleted the mergelatest branch August 20, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.