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

Numpad keypress not logging #1

Conversation

mariomurrent-softwaresolutions
Copy link

@mariomurrent-softwaresolutions mariomurrent-softwaresolutions commented Jan 6, 2021

Merged changes from React-Native master into numpad-keypress-not-logging branch
@fabriziobertoglio1987

@mariomurrent-softwaresolutions
Copy link
Author

@fabriziobertoglio1987 any updates? 😊

fabOnReact pushed a commit that referenced this pull request Feb 1, 2021
Summary:
changelog: [internal]

Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t

# Why is using size_t better when working with indexes.

## 1. Type conversion isn't for free.

Take this example

```
size_t calculate(int number) {
  return number + 1;
}
```

It generates following assembly (generated with armv8-a clang 10.0.0):

```
calculate(int):                          // calculate(int)
sub     sp, sp, facebook#16                     // =16
str     w0, [sp, facebook#12]
ldr     w8, [sp, facebook#12]
add     w9, w8, #1                      // =1
mov     w8, w9
sxtw    x0, w8
add     sp, sp, facebook#16                     // =16
ret
```

That's 9 instructions.

If we get rid of type conversion:

```
size_t calculate(size_t number) {
  return number + 1;
}
```

Assembly (generated with armv8-a clang 10.0.0):

```
calculate(unsigned long):                          // calculate(unsigned long)
sub     sp, sp, facebook#16             // =16
str     x0, [sp, #8]
ldr     x8, [sp, #8]
add     x0, x8, #1              // =1
add     sp, sp, facebook#16             // =16
ret
```

Compiler now produces only 7 instructions.

## Semantics

When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.

Reviewed By: JoshuaGross

Differential Revision: D24332248

fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
@fabOnReact
Copy link
Owner

@mariomurrent-softwaresolutions thanks a lot. Yes, you are right. I should push the master branch more often, but usually I use my local version .. because I'm busy.

I can not merge this pr ... sorry.. I am very busy right now and this fork is not meant to be used for collaboration.
But if you are willing to sponsor me up to 250 USD per month, I will consider publishing my own version of react-native and will use the fork to keep track of issues https://github.com/sponsors/fabriziobertoglio1987

Thanks a lot for the advice 🙏

@fabOnReact fabOnReact closed this Mar 3, 2021
fabOnReact pushed a commit that referenced this pull request Nov 29, 2023
Summary:
Pull Request resolved: facebook#41466

## Context
In open source, all apps use the same turbomodulemanager delegate (i.e: the default delegate).

This diff introduces the buck infra that makes the oss default delegate work for meta apps.

Concretely, we are going to make React Native use the same  delegate for **all** Meta apps.

Each Meta app will:
1. At build time, generate a unique TMProvider map
2. At app init time, initialize the default delegate with the TMProvider map.

## Implementation
**Step #1:** At build time, generate a unique TMProvider map

**Insight:** Buck genrules can accept, as input, the output of a buck query.

So, here's how we get this done:
1. Buck query (i.e: input to Genrule): Given the app's deps, query all the schemas in the app.
2. Genrule: Read the schemas to generate the TMProvider map. The TMProvider map will also contain **all** the app's C++ module codegen.

Concretely:
1. This diff introduces a macro: rn_codegen_appmodules(deps).
2. rn_codegen_appmodules(deps) generates appmodules.so, which contains the TMProvider map.

**Step #2:** At app init time, initialize the default delegate with the TMProvider map.

This is how we'll initialize the DefaultTurboModuleManagerDelegate:
1. DefaultTurboModuleManagerDelegate will load appmodules.so during init.
2. When loaded, appmodules.so will assign the code-generated TMProvider map to DefaultTurboModuleManagerDelegate.

## Impact
This should allow us to:
1. Get one step closer to getting rid of the `js1 build turbomodule-manager-delegates --target <app>` script
3. Remove the TurboModuleManagerDelegate from React Native's public API. (Because we use one delegate for all React Native apps in Meta and OSS)

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D50988397

fbshipit-source-id: 0ca5dec14e2dae89ec97f5d39a182c7937c5c7bf
fabOnReact pushed a commit that referenced this pull request Feb 18, 2024
…gets

Summary:
Changelog: [Internal]

# This diff

1. Provides all Targets with an `executorFromThis()` method, which can be used from within a Target to access a *`this`-scoped main thread executor* = a `std::function` that will execute a callback asynchronously iff the current Target isn't destroyed first.
2. Refactors how (all) Target objects are constructed and retained, from a plain constructor to `static shared_ptr create()`. This is because `executorFromThis()` relies internally on `enable_shared_from_this` plus two-phase construction to populate the executor.
3. Creates utilities for deriving scoped executors from other executors and `shared_ptr`s.

The concept is very much like `RuntimeExecutor` in reverse: the #1 use case is moving from the JS thread back to the main thread - where "main thread" is defined loosely as "anywhere it's legal to call methods on Target/Agent objects, access session state, etc". The actual dispatching mechanism is left up to the owner of each `PageTarget` object; for now we only have an iOS integration, where we use `RCTExecuteOnMainQueue`.

Coupling the ownership/lifetime semantics with task scheduling is helpful, because it avoids the footgun of accidentally/nondeterministically moving `shared_ptr`s (and destructors!) to a different thread/queue .

# This stack
I'm refactoring the way the Runtime concept works in the modern CDP backend to bring it in line with the Page/Instance concepts.

Overall, this will let us:

* Integrate with engines that require us to instantiate a shared Target-like object (e.g. Hermes AsyncDebuggingAPI) in addition to an per-session Agent-like object.
* Access JSI in a CDP context (both at target setup/teardown time and during a CDP session) to implement our own engine-agnostic functionality (`console` interception, `Runtime.addBinding`, etc).
* Manage CDP execution contexts natively in RN, and (down the line) enable first-class debugging support for multiple Runtimes in an Instance.

The core diffs in this stack:

* ~~Introduce a `RuntimeTarget` class similar to `{Page,Instance}Target`. ~~
* ~~Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`). ~~
* ~~Rename the existing `RuntimeAgent` interface to `RuntimeAgentDelegate`.~~
* ~~Create a new concrete `RuntimeAgent` class similar to `{Page,Instance}Agent`.~~
* ~~Provide `RuntimeTarget` and `RuntimeAgent` with primitives for safe JSI access, namely a `RuntimeExecutor` for scheduling work on the JS thread.~~
  * Provide RuntimeTarget with mechanism for scheduling work on the "main" thread from the JS thread, for when we need to do more than just send a CDP message (which we can already do with the thread-safe `FrontendChannel`) in response to a JS event. *← This diff*

## Architecture diagrams

Before this stack:
https://pxl.cl/4h7m0

After this stack:
https://pxl.cl/4h7m7

Reviewed By: hoxyq

Differential Revision: D53356953

fbshipit-source-id: 152c784eb64e9b217fc2966743b33f61bd8fd97e
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.

2 participants