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

Idea: always freeze + freeze export #687

Closed
mweststrate opened this issue Oct 21, 2020 · 4 comments
Closed

Idea: always freeze + freeze export #687

mweststrate opened this issue Oct 21, 2020 · 4 comments

Comments

@mweststrate
Copy link
Collaborator

🚀 Feature Proposal

Currently autoFreeze is disabled by default in production builds, with the assumption that that is faster. However, since the deep pruning for drafts introduced in Immer 7, that is no longer the case; frozen data allows for an early bail out, where non frozen data doesnt. So although initial storage is slower with autofreeze, subsequent updates are faster.

By enabling auttoFreeze in all environments by default we fix a few problems:

  • no difference between prod and non prod
  • usually faster, but always more predictable performance

As optimisation tip it should be made clear that shallowly pre-freezing data about to be stored can bail out of expensive deep freezing without loosing the benefits of the non-deep pruning. Maybe even export a specific preFreeze method for this reason, or reuse the existing freeze export.

@ctrlplusb
Copy link

@mweststrate if you are to make this change can I please ask that you maintain the ability to override this setting manually still - both globally or via the Immer class constructor. 🙏

Would someone be able to point me in the direction of docs/discussions around the reasoning and behaviour of the deep pruning? x

@mweststrate
Copy link
Collaborator Author

@ctrlplusb yes, it will still be possible to use setAutoFreeze. I don't think there is a good summary of the discussion anywhere, but the TL,DR is: since Immer 6, Immer might do a full tree traversal of any data stored using produce; either to freeze it, or to see if there are any pending drafts.

  1. If auto freezing is enabled, there will only be a data traversal initially when storing new data into the state tree to freeze it
  2. If data is not frozen, on any subsequent produce accessing that data, Immer might potentially do a full traversal to see if there were any mutations in that part of the tree (often it doesn't, but it might)
  3. So whether freezing by default is faster or slower will depend on individual circumstances, but at least by always freezing recursively by default, the performance is
    1. more predictable
    2. and the same between prod and dev environments. The old behavior could result in both big speed ups or slow downs in prod environments, see Immer 7 is 8 times slower than immer 6 #681 for a nice example, where not freezing made things 8 times slower.
    3. A nice side effect is that accidental mutations will be caught in prod just as they are in dev
  4. To prevent a full deep freeze of data that you know won't be touched in the future (for example events that are stored over time, but that will never change), you can optimize this by using freeze(data) to shallowly freeze data before storing it, e.g. draft.push(freeze(event)). That was already a valid optimization before, but again with the new defaults this means that these kind of optimizations do properly reflect what will happen in production as well.

@ctrlplusb
Copy link

I appreciate the insight 💜

mweststrate added a commit that referenced this issue Nov 17, 2020
BREAKING CHANGE: always freeze by default, even in production mode. Use `setAutoFreeze(process.env.NODE_ENV !== 'production')` for the old behavior. See #687 (comment) for the rationale. Fixes #649, #681, #687
@mweststrate
Copy link
Collaborator Author

Released in 8.0.0

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

No branches or pull requests

2 participants