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

Breaking: Remove YGConfigGetInstanceCount #1370

Closed
wants to merge 4 commits into from

Conversation

NickGerleman
Copy link
Contributor

Summary:
This was added in #497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: 6d6dfec415bef9d2fc37e4ac776dfe62868b4839
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: a3b6a5308571ac8d2332f6f55c5f5d8f1993a45b
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: 243b3780e7a2fdf8b63ccdd67fc1b10b39198d67
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: 3444bfe067cb0ac55b92952a3635ba2d8a4b8ae4
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: f62a6d802f2da6ee6196043cdc16392d23ad67cb
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: f13348bc3ead322105c6a7dcfda167a9b13a821d
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: 80ae77ab6312a86eaa3046061406318d12a2d6c2
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Differential Revision: D49131207

fbshipit-source-id: 2c792751af5b31354a06ff3985040a73691b8009
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 11, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 4c66f4e0692ff579ce4d8ee38e2f1176977a766d
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 11, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 6e59be2a10d9de4c48ffc9174fd075d3075d1e53
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: fa732ba593e6d1bc97c0274a73b03626d25cd556
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 67f665ead7aa7690dc2a24a4c1a26b68841bee80
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 1e95a2e55076c5916d5df45e65ee873e7a7afe4a
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 160d2f0d33cfa224b495b811d0c526aa0be6b0de
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 80068755141b0c9e2bb005d336020d8783b4fb01
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: dac73b936f38071a838d2135d298706be11128bb
NickGerleman and others added 4 commits September 11, 2023 19:00
Summary:
This changes public Yoga API to in more places accept const structures where before they required mutable ones.

This tries to avoid more breaking changes yet, e.g. changing callbacks to require clients do not modify nodes when they are passed for logging. We also don't have const variants for returning child structures which would allow mutation of dependencies of the const object. These would need new names under the public API, since we do not have operator overloading in C.

Differential Revision: D49130412

fbshipit-source-id: ee02eeb1d9e6fcb299da1655a791dd599350c67c
Summary:
Pull Request resolved: facebook#1369

X-link: facebook/react-native#39370

This fixes const-correctness of callbacks (e.g. not letting a logger function modify nodes during layout). This helps us to continue to fix const-correctness issues inside of Yoga.

This change is breaking to the public API, since it requires a change in signature passed to Yoga.

Changelog: [Internal]

Differential Revision: https://internalfb.com/D49130714

fbshipit-source-id: a01ca31e172350fd62e04350f99e412035a70b61
Summary:
Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Differential Revision: D49130914

fbshipit-source-id: 08d1d7dfd883c4adcc736ed38b7f2ac1482140a6
Summary:
Pull Request resolved: facebook#1370

X-link: facebook/react-native#39369

This was added in facebook#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 4e0f15068a10f95597e3421d3e7ad84d840eec94
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: facebook#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 36cf4caebaba498a806c0aeea853b54f18077832
facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1370

X-link: facebook/react-native#39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 58537ed635ed455ff065471bdf77061a4bf826f4
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1370

Pull Request resolved: #39369

This was added in facebook/yoga#497 specifically for tests related to memory leaks in the C# bindings to count how often YGConfigFree.

This is the wrong layer for this check, we don't have officially supported C# bindings anymore, and this API is not safe when Yoga runs on multiple threads. This removes it, similar to a global node instance count that was also previously removed.

Reviewed By: rshest

Differential Revision: D49131207

fbshipit-source-id: 58537ed635ed455ff065471bdf77061a4bf826f4
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a2d3fc6.

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

Successfully merging this pull request may close these issues.

2 participants