Skip to content

Commit

Permalink
Remove dummyGlobal (software-mansion#3838)
Browse files Browse the repository at this point in the history
## Summary

This PR removes a piece of code in `RuntimeDecorator` that used to overwrite `global` property of the global JS object with a dummy JS object which was a regular JS object filled only with Reanimated-specific values.

## Motivation

I tried to confirm that Hermes was indeed enabled on the UI context with the following snippet:
```ts
runOnUI(() => {
  'worklet';
  console.log('HermesInternal' in global); // should return true
})();
```
However, it returned `false`, despite the fact that a breakpoint on `facebook::hermes::makeHermesRuntime` was hit.

Then I noticed that we call `RuntimeDecorator::decorateRuntime(rt, "UI");` which overwrites `global` property in the UI context with a "dummy global", effectively causing `global.HermesInternal` to be `undefined`. 

## Context

The concept of dummy global was introduced in the following PR:

* software-mansion#882

There was one attempt to remove dummy global in the past but the change was immediately reverted in f846704:

* software-mansion#2253

As @wkozyra95 stated:

> removed dummyGlobal and make `global.global` point to itself. I don't necessarily need that change to make it work, but the current behavior of global value seems incorrect

## Changes

One might wonder why not just remove these three lines:

https://github.com/software-mansion/react-native-reanimated/blob/074cac02d2ef503e7897ef63560d3b8514a8e350/Common/cpp/Tools/RuntimeDecorator.cpp#L33-L35

I'm glad you asked. The answer is that is simply doesn't work:

<img width="250" alt="Zrzut ekranu 2022-12-6 o 11 27 08" src="https://user-images.githubusercontent.com/20516055/205896462-1616af17-070d-43a9-9231-1bf4c6cb3e24.png">

That's because in many places we use `global.foo` syntax where `global` most likely represents the `global` property of the global object, not the global object itself (it's complicated).

The solution is to add the global JS object as the `global` property of itself so that `global.global === global`.

Obviously, it's like a reference cycle but hopefully the runtime knows how to clean itself once it gets terminated.

## What about ...?

### Overriding runtime global

Snippet:

```tsx
import React from 'react';
import { runOnUI } from 'react-native-reanimated';

export default function App() {
  // JS
  console.log('HermesInternal' in global);

  runOnUI(() => {
    'worklet';
    // UI
    console.log('HermesInternal' in global);
  })();

  return <></>;
}
```

Before:

![before2](https://user-images.githubusercontent.com/20516055/206486688-d6ac80fb-66b7-4a9d-8134-64895efd7c0f.png)

After:

![after2](https://user-images.githubusercontent.com/20516055/206486664-1fb9e786-9e42-45c2-bb4b-1131e0a9f616.png)

### Runtime deallocation

After the changes, the runtime is deallocated on app reload:

<img width="1103" alt="dealloc" src="https://user-images.githubusercontent.com/20516055/205902815-8087572d-33ff-4909-901d-39458f68ac73.png">

### Capturing global in worklets

Should work the same way since `plugin.js` hasn't been changed.

Input (from software-mansion#882):

```ts
function out(easing) {
  'worklet';
  return t => {
    'worklet';
    return 1 - easing(1 - t);
  };
}
```

Command: `npx babel file.js`

Output (after formatting):

```js
var out = (function () {
  var _f = function _f(easing) {
    return (function () {
      var _f = function _f(t) {
        return 1 - easing(1 - t);
      };
      _f._closure = { easing: easing };
      _f.asString =
        'function _f(t) {\n  const {\n    easing\n  } = this._closure;\n  return 1 - easing(1 - t);\n}\n//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJtYXBwaW5ncyI6IkFBRVNBLFNBQUNDLEVBQURELENBQUNBLENBQURBLEVBQUs7QUFBQTtBQUFBO0FBQUE7QUFFVixTQUFPLElBQUlFLE1BQU0sQ0FBQyxJQUFJRixDQUFMLENBQWpCO0FBRktBIiwibmFtZXMiOlsidCIsIl9mIiwiZWFzaW5nIl0sInNvdXJjZXMiOlsiL1VzZXJzL3RvbWVremF3L1JOT1MvU2hhcmVhYmxlUmV3cml0ZS9yZWFjdC1uYXRpdmUtcmVhbmltYXRlZC9maWxlLmpzIl0sInNvdXJjZXNDb250ZW50IjpbImZ1bmN0aW9uIG91dChlYXNpbmcpIHtcbiAgJ3dvcmtsZXQnO1xuICByZXR1cm4gdCA9PiB7XG4gICAgJ3dvcmtsZXQnO1xuICAgIHJldHVybiAxIC0gZWFzaW5nKDEgLSB0KTtcbiAgfTtcbn0iXX0=';
      _f.__workletHash = 8053949848116;
      _f.__location =
        '/Users/tomekzaw/RNOS/ShareableRewrite/react-native-reanimated/file.js (3:9)';
      return _f;
    })();
  };
  _f._closure = {};
  _f.asString =
    "function out(easing) {\n  return function (t) {\n    'worklet';\n\n    return 1 - easing(1 - t);\n  };\n}\n//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJtYXBwaW5ncyI6IkFBQUE7QUFFRSxTQUFPQSxhQUFLO0FBQ1Y7O0FBQ0EsV0FBTyxJQUFJQyxNQUFNLENBQUMsSUFBSUQsQ0FBTCxDQUFqQjtBQUZGO0FBRkYiLCJuYW1lcyI6WyJ0IiwiZWFzaW5nIl0sInNvdXJjZXMiOlsiL1VzZXJzL3RvbWVremF3L1JOT1MvU2hhcmVhYmxlUmV3cml0ZS9yZWFjdC1uYXRpdmUtcmVhbmltYXRlZC9maWxlLmpzIl0sInNvdXJjZXNDb250ZW50IjpbImZ1bmN0aW9uIG91dChlYXNpbmcpIHtcbiAgJ3dvcmtsZXQnO1xuICByZXR1cm4gdCA9PiB7XG4gICAgJ3dvcmtsZXQnO1xuICAgIHJldHVybiAxIC0gZWFzaW5nKDEgLSB0KTtcbiAgfTtcbn0iXX0=";
  _f.__workletHash = 3984642990765;
  _f.__location =
    '/Users/tomekzaw/RNOS/ShareableRewrite/react-native-reanimated/file.js (1:0)';
  return _f;
})();
```

Conclusion: `global` is not captured (as `_f._closure = {};`), which is correct.

### `gc`

Snippet:

```tsx
import React from 'react';
import { runOnUI } from 'react-native-reanimated';

export default function App() {
  // JS
  console.log(gc);
  console.log(gc());
  console.log(global.gc);
  console.log(global.gc());

  runOnUI(() => {
    'worklet';
    // UI
    console.log(gc);
    // console.log(gc()); // Tried to synchronously call a non-worklet function on the UI thread.
    console.log(global.gc);
    console.log(global.gc());
  })();

  return <></>;
}
```

Before:

![before](https://user-images.githubusercontent.com/20516055/205899206-5736176c-3568-4697-bc09-c97ff6d67405.png)

After:

![after](https://user-images.githubusercontent.com/20516055/205899489-491357fe-e0f3-4ab9-8102-120f7e864383.png)

## Test plan

Just check the above test cases one-by-one.

## TODO

- [ ] check if runtime deallocates on reload (set a breakpoint in `~JSCRuntime`)
- [ ] check if `global` is not in closure
- [ ] check if `gc` works
- [ ] check if expo-gl works
  • Loading branch information
tomekzaw authored and fluiddot committed Jun 5, 2023
1 parent 84b13a6 commit 93dd9a8
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions Common/cpp/Tools/RuntimeDecorator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ void RuntimeDecorator::decorateRuntime(
rt.global().setProperty(
rt, "_LABEL", jsi::String::createFromAscii(rt, label));

jsi::Object dummyGlobal(rt);
dummyGlobal.setProperty(rt, "gc", rt.global().getProperty(rt, "gc"));
rt.global().setProperty(rt, "global", dummyGlobal);
rt.global().setProperty(rt, "global", rt.global());

rt.global().setProperty(rt, "jsThis", jsi::Value::undefined());

Expand Down

0 comments on commit 93dd9a8

Please sign in to comment.