-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Attach pointer to worklet runtime in main runtime #2253
Merged
Szymon20000
merged 2 commits into
software-mansion:master
from
wkozyra95:support-for-expo-gl
Aug 4, 2021
Merged
Attach pointer to worklet runtime in main runtime #2253
Szymon20000
merged 2 commits into
software-mansion:master
from
wkozyra95:support-for-expo-gl
Aug 4, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Szymon20000
approved these changes
Aug 4, 2021
6 tasks
piaskowyk
pushed a commit
that referenced
this pull request
Nov 25, 2021
## Description When I was adding #2253 I forgot about iOS part, this PR injects the same value as on android ## Changes For example: - Added `foo` method which add bouncing animation - Updated `about.md` docs - Added caching in CI builds
piaskowyk
pushed a commit
that referenced
this pull request
Dec 8, 2022
## 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: * #882 There was one attempt to remove dummy global in the past but the change was immediately reverted in f846704: * #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:  After:  ### 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 #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:  After:  ## 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
fluiddot
pushed a commit
to wordpress-mobile/react-native-reanimated
that referenced
this pull request
Jun 5, 2023
## 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:  After:  ### 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:  After:  ## 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We want to add support for expo-gl in worklets,
Changes
global.global
point to itself. I don't necessarily need that change to make it work, but the current behavior of global value seems incorrectTest code and steps to reproduce
I prepared 2 examples here wkozyra95@b3618a8
I tested this example yarn linking modified
expo-gl-cpp
, so it does not work in the form with latest version of that package.Checklist