refactor: well-defined behaviour #52187

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.

Sign up for GitHub

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

Jump to bottom

Closed

hoxyq wants to merge 1 commit into facebook:main from hoxyq:export-D77053030

+46 −41

Contributor

hoxyq commented Jun 23, 2025

Summary:

Changelog: [Internal]

tracing_ is now std::atomic<bool>.
performanceMeasureCount_ is nowstd::atomic<uint32_t>.
mutex_ -> bufferMutex_.

The biggest change is that we no longer read from tracing_ directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

facebook-github-bot added CLA Signed p: Facebook Partner labels

Contributor

facebook-github-bot commented Jun 23, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

facebook-github-bot added the fb-exported label

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

eb91d2e

Summary:

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `bufferMutex_`.


The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from 3572790 to 7399b64 Compare

June 23, 2025 12:06

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

7399b64

Summary:

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `bufferMutex_`.


The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

Contributor

facebook-github-bot commented Jun 23, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

e9e295b

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `bufferMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from 7399b64 to e9e295b Compare

June 23, 2025 12:09

Contributor

facebook-github-bot commented Jun 23, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

c78c2fc

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

485fce7

Summary:

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.


The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from e9e295b to 485fce7 Compare

June 23, 2025 13:36

Contributor

facebook-github-bot commented Jun 23, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

c864d01

Summary:

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.


The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from 485fce7 to c864d01 Compare

June 23, 2025 14:12

Contributor

facebook-github-bot commented Jun 23, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from c864d01 to 07dc425 Compare

June 23, 2025 14:16

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

07dc425

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

Contributor

facebook-github-bot commented Jun 23, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

2bd5ba6

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

8bf29ed

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

b178e33

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from 07dc425 to ee13d92 Compare

June 24, 2025 20:47

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

ee13d92

Summary:

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Differential Revision: D77053030

Contributor

facebook-github-bot commented Jun 24, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

c468599

Summary:

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from ee13d92 to c468599 Compare

June 24, 2025 22:33

Contributor

facebook-github-bot commented Jun 24, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

38acf55

Summary:

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Reviewed By: rubennorte

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

b04d020

Summary:

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Reviewed By: rubennorte

Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch 2 times, most recently from b04d020 to 286d96e Compare

June 25, 2025 09:37

Contributor

facebook-github-bot commented Jun 25, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

60c6211

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

8fc6d2d

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

Contributor

facebook-github-bot commented Jun 25, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030


          refactor: well-defined behaviour (facebook#52187)

c6c2d10

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Reviewed By: rubennorte

Differential Revision: D77053030

Contributor

facebook-github-bot commented Jun 25, 2025

This pull request was exported from Phabricator. Differential Revision: D77053030

hoxyq force-pushed the export-D77053030 branch from 286d96e to c6c2d10 Compare

June 25, 2025 09:43

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

77aa278

Summary:

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Reviewed By: rubennorte

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

1c2954d

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

7ff7a39

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

9ca3a01

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

2fcc403

Summary:

# Changelog: [Internal]

- `bool tracing_` -> `std::atomic<bool> tracingAtomic_`.
- More doc-comments to explain the usage of mutex and atomics.
- `PerformanceTracer::isTracing()` -> `inline PerformanceTracer::isTracing()`.
- `uint64_t processId_` -> `const uint64_t processId_`.

The main change is that the boolean flag that controls "if we are tracing" is now atomic, which should eliminate potential data races. To avoid "logic" races, we are still going to lock mutex, and then check again. The use of `std::atomic` allows us to perform cheaper check first to avoid potentially unnecessary serializations from other systems that report events into `PerformanceTracer`.

Reviewed By: rubennorte

Differential Revision: D77053030

hoxyq added a commit to hoxyq/react-native that referenced this pull request


          refactor: well-defined behaviour (facebook#52187)

6bc87b3

Summary:
Pull Request resolved: facebook#52187

# Changelog: [Internal]

- `tracing_` is now `std::atomic<bool>`.
- `performanceMeasureCount_` is now`std::atomic<uint32_t>`.
- `mutex_` -> `tracingMutex_`.

The biggest change is that we no longer read from `tracing_` directly for an early-return when not tracing. This should not significantly regress performance of the subsystem, see the test plan for numbers.

Differential Revision: D77053030

facebook-github-bot closed this in

e504909

Collaborator

react-native-bot commented Jun 25, 2025

This pull request was successfully merged by @hoxyq in e504909

^{When will my fix make it into a release? | How to file a pick request?}

react-native-bot added the Merged label

Contributor

facebook-github-bot commented Jun 25, 2025

This pull request has been merged in e504909.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed fb-exported Merged p: Facebook Partner