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

Add experimental new memoizers: autotrack and weakmap #605

Merged
merged 9 commits into from
May 10, 2023

Conversation

markerikson
Copy link
Contributor

@markerikson markerikson commented Apr 12, 2023

This PR:

There's a good chance that these could go into v4.x, but I'm putting them in a 5.x-based build branch for now.

Status and Tradeoffs

Currently, autotrack and weakMap seem to actually work and pass tests. signalis runs but doesn't memoize properly.

autotrack works well! I can confirm that if you do something like todos => todos.map(t => t.id), and then flip t.completed, it will not recalculate. However, it also is definitely slower than defaultMemoize. The question is whether the slowness is enough to matter in practice. On the flip side, the fact that it can recalculate a lot less often in some cases means that there'd be fewer re-renders.

weakMap is closer in speed to defaultMemoize. Haven't done significant testing on it yet. Seems like there's some amount of multi-size behavior by default, but haven't figured out how that grows over time or gets GC'd.

Resources / inspiration / places I swiped code from:

@markerikson
Copy link
Contributor Author

Aaaand the build is totally busted due to some Reselect -> RTK -> Reselect dependency thing.

Will worry about that later.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 18, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 41959cd:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@markerikson markerikson force-pushed the feature/5.0-experimental-memoizers branch 2 times, most recently from 8c62e90 to b644c27 Compare April 18, 2023 18:26
@markerikson markerikson force-pushed the feature/5.0-experimental-memoizers branch 2 times, most recently from 1b61862 to 2d43549 Compare April 27, 2023 01:19
@markerikson markerikson changed the title Add experimental new memoizers: autotrack, weakmap, and signalis Add experimental new memoizers: autotrack, weakmap, ~~and signalis~~ May 10, 2023
@markerikson markerikson changed the title Add experimental new memoizers: autotrack, weakmap, ~~and signalis~~ Add experimental new memoizers: autotrack and weakmap May 10, 2023
@markerikson markerikson force-pushed the feature/5.0-experimental-memoizers branch from b5499d6 to 499b734 Compare May 10, 2023 03:09
@markerikson markerikson force-pushed the feature/5.0-experimental-memoizers branch from c7abf78 to 7efdeea Compare May 10, 2023 03:22
@markerikson markerikson marked this pull request as ready for review May 10, 2023 03:36
@markerikson
Copy link
Contributor Author

Updates since the original PR creation:

  • Deleted the signalisMemoize implementation. It didn't work right.
  • Cleaned up autotrackMemoize and weakmapMemoize
  • Did some testing on weakmapMemoize in conjunction with @crutchcorn and concluded that weakmapMemoize has effectively an infinite cache size, in that the use of WeakMaps will keep around cached values as long as the args are still in memory, and then clean up the cached values as soon as those args are GC'd.
  • Split the unit tests into several files just for readability, and to enable adding copy-pastes of the "basic selector" tests using both autotrackMemoize and weakmapMemoize

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.

1 participant