Optimize snapshot cloning overhead for callbacks #1533
Closed
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
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: ccc654763404e87ad5534114767f36c2dc752f39
drarmstr
force-pushed
the
export-D33490900
branch
from
d86c2e1
to
a2673c2
Compare
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: 787b7aede8979ec3a8cddff8a4c03a80b0dce6a4
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 89e1acb5f320959d6c821d7766721366772d460a
drarmstr
force-pushed
the
export-D33490900
branch
from
a2673c2
to
659a5c3
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 8539d17b7b5ac213f40480f4788d2adba6304cdd
drarmstr
force-pushed
the
export-D33490900
branch
from
659a5c3
to
24a57ef
Compare
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: https://www.internalfb.com/diff/D33490900?entry_point=27 fbshipit-source-id: f86f64c0e801c34b7af0ce89a1b141e0d193cc50
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: https://www.internalfb.com/diff/D33490900?entry_point=27 fbshipit-source-id: 06018af1f383816428f7813f0dc8585e36f280a4
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 8, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: https://www.internalfb.com/diff/D33490900?entry_point=27 fbshipit-source-id: 9d05884e9d054e2a0b68501df0f1b43ff2bb741b
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 12, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 09a8504615a71ba4be3dd4e36c1f1dfe873621e1
drarmstr
force-pushed
the
export-D33490900
branch
from
24a57ef
to
a221254
Compare
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: 44f0aed51e5d61701e878c3b593acff87655221a
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: e78014c0fed6e404f7debcf8fa58f5f6f94805eb
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: ed72704df156aa796478d560ab20ed456d84a73f
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: 0ff9ea37be80de272ca529fdf0da1912fb7636f2
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: 5a3bb5d54438ca3c0690d7eddb73dddb504f6845
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: b7326c6cf0d3a90565ddabca1bddd5e313e225b9
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 2c6cc370c672a6e4b016ce31d8b49eedfb1c9ce5
drarmstr
force-pushed
the
export-D33490900
branch
from
a221254
to
fd14b21
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: 592ac8edf75b41ab899821939a0f7231b2262474
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 39737c4ea69724939beb390089a62fdbbedadd4c
drarmstr
force-pushed
the
export-D33490900
branch
from
fd14b21
to
8269522
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: c1d1faa97dea0c55de7b51f93d605b015e4eaf3e
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
force-pushed
the
export-D33490900
branch
from
8269522
to
a225d2a
Compare
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 9080ec65b945b7fa9f53154b683ab5c055cd3338
drarmstr
force-pushed
the
export-D33490900
branch
from
a225d2a
to
4f830c3
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Differential Revision: D33490900 fbshipit-source-id: 0c82999b0cbf53ccea8ed61f6c44cb741ed468ce
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 13, 2022
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 41cad9a396f9b8aaabb309656775ea2f4abefe3c
drarmstr
force-pushed
the
export-D33490900
branch
from
4f830c3
to
60cfcac
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
Summary: Pull Request resolved: facebookexperimental#1545 With us relying more on accurate snapshot retention I tracked down where we were getting warnings about using released snapshots to the RecoilDevTools `<Connector>` component. * Properly retain the snapshot until it is used in the effect for tracking. While snapshots are retained until an effect releases them, that auto-release effect was running before this tracking effect. * Also cleanup the `useEffect()` to have a proper deps array and support proper cleanup and re-connecting. Remove setting an `initialSnapshot` as it would have caused the effect to re-connect each time. Curiously, re-connecting casuses new state changes to not appear in the dev-tools, so the `<Connector>` may not yet be compatible with `<StrictMode>`. Differential Revision: https://www.internalfb.com/diff/D33559247?entry_point=27 fbshipit-source-id: 5bfed7ffb938a46033445fff09d42b351cfc0daf
…tal#1546) Summary: Pull Request resolved: facebookexperimental#1546 Add an `isRetained()` method for Snapshots and do a safety check in `retain()` to make sure we are not trying to retain snapshots that have already been released. Differential Revision: D33559535 fbshipit-source-id: f4560b46468f7fe9529bcbaa304d99994d9b4ef7
…l#1533) Summary: Pull Request resolved: facebookexperimental#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 8d69e3bf3dc7da94e64adc5d58b604ea8aea0ad2
drarmstr
force-pushed
the
export-D33490900
branch
from
60cfcac
to
8b3cd1c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33490900 |
AlexGuz23
pushed a commit
to AlexGuz23/Recoil
that referenced
this pull request
Nov 3, 2022
Summary: Pull Request resolved: facebookexperimental/Recoil#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 093b2aa498337bb2abc8eebe54a72e1a130b3469
snipershooter0701
pushed a commit
to snipershooter0701/Recoil
that referenced
this pull request
Mar 5, 2023
Summary: Pull Request resolved: facebookexperimental/Recoil#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 093b2aa498337bb2abc8eebe54a72e1a130b3469
eagle2722
added a commit
to eagle2722/Recoil
that referenced
this pull request
Sep 21, 2024
Summary: Pull Request resolved: facebookexperimental/Recoil#1533 If a user has a recoil callback, such as from `useRecoilCallback()`, and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed. Reviewed By: habond Differential Revision: D33490900 fbshipit-source-id: 093b2aa498337bb2abc8eebe54a72e1a130b3469
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.
Summary: If a user has a recoil callback, such as from
useRecoilCallback(), and uses it to get a snapshot many times in a loop it could incur overhead for cloning the snapshot. Optimize this by caching the cloned snapshot and only cloning a new one if the store or state has changed.Differential Revision: D33490900