Skip to content

Put mounting refs in a queue#3493

Closed
JoviDeCroock wants to merge 10 commits into
v11from
queue-mounting-refs
Closed

Put mounting refs in a queue#3493
JoviDeCroock wants to merge 10 commits into
v11from
queue-mounting-refs

Conversation

@JoviDeCroock

@JoviDeCroock JoviDeCroock commented Mar 27, 2022

Copy link
Copy Markdown
Member

Issues with refs

I kinda get the issues with refs now, it basically ties into how we can switch from a patch oriented diff to a mount one.

In essence we always discard refs as part of our unmount process which makes sense in essence but this could prove to be hard when we do

    const App = ({ open }) =>
      open ? (
        <div class=“open” key=“open”>
          <div ref={ref} />
        </div>
      ) : (
        <div class=“closes” key=“closed”>
          <div ref={ref} />
        </div>
      );

Now we put either one on the unmount queue and switch to mount-mode for the other. Which essentially means that we dive into mount which immediately applies the ref as part of mountChildren and when we trampoline back up into patchChildren we will meet the unmount call and erase the ref to null.

The issue with queueing is determining the timeframe to apply the refs as they will need to be there before our layout-effects are triggering so we need it before we connect the new nodes to our root-tree.

This makes a queue a hard task as we try to be as indeterminate as possible about what phase we are in.

On the bright side we could start experimenting with tracking the refs on the rendererState still a bit hesitant on when to start applying them.

The thing that worries me about the patch —> mount cycle is that we would probably even here have issues queuing and keeping a deterministic order. i.e. we could keep two arrays with nullish and non-nullish invocations, this would result in us executing them out of order —> all null —> all value rather than null - value - null - value (useImperativeHandle). I think we would just need to find a special case for this replacement logic.

@github-actions

github-actions Bot commented Mar 27, 2022

Copy link
Copy Markdown

📊 Tachometer Benchmark Results

Summary

duration

  • 02_replace1k: unsure 🔍 -1% - +1% (-1.03ms - +0.93ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: unsure 🔍 -1% - +5% (-0.50ms - +2.89ms)
    preact-local vs preact-master
  • 07_create10k: unsure 🔍 -0% - +1% (-6.16ms - +8.59ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 -3% - +0% (-9.17ms - +0.58ms)
    preact-local vs preact-master
  • hydrate1k: unsure 🔍 -7% - +10% (-10.48ms - +15.05ms)
    preact-local vs preact-master
  • many_updates: unsure 🔍 -5% - +2% (-11.68ms - +3.87ms)
    preact-local vs preact-master
  • text_update: unsure 🔍 -0% - +1% (-0.11ms - +0.85ms)
    preact-local vs preact-master
  • todo: unsure 🔍 -4% - +3% (-2.19ms - +1.54ms)
    preact-local vs preact-master

usedJSHeapSize

  • 02_replace1k: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
    preact-local vs preact-master
  • 07_create10k: unsure 🔍 -0% - +0% (-0.10ms - +0.06ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
    preact-local vs preact-master
  • hydrate1k: unsure 🔍 +0% - +0% (+0.00ms - +0.01ms)
    preact-local vs preact-master
  • many_updates: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
    preact-local vs preact-master
  • text_update: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
    preact-local vs preact-master
  • todo: unsure 🔍 -2% - +3% (-0.03ms - +0.04ms)
    preact-local vs preact-master

Results

02_replace1k

duration

VersionAvg timevs preact-mastervs preact-local
preact-master115.28ms - 116.75ms-unsure 🔍
-1% - +1%
-0.93ms - +1.03ms
preact-local115.32ms - 116.61msunsure 🔍
-1% - +1%
-1.03ms - +0.93ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master4.16ms - 4.17ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms
preact-local4.17ms - 4.18msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms
-

run-warmup-0

VersionAvg timevs preact-mastervs preact-local
preact-master46.12ms - 47.99ms-unsure 🔍
-2% - +3%
-1.02ms - +1.34ms
preact-local46.17ms - 47.61msunsure 🔍
-3% - +2%
-1.34ms - +1.02ms
-

run-warmup-1

VersionAvg timevs preact-mastervs preact-local
preact-master56.57ms - 58.13ms-unsure 🔍
-2% - +2%
-1.02ms - +1.30ms
preact-local56.35ms - 58.06msunsure 🔍
-2% - +2%
-1.30ms - +1.02ms
-

run-warmup-2

VersionAvg timevs preact-mastervs preact-local
preact-master50.44ms - 53.01ms-unsure 🔍
-4% - +3%
-2.21ms - +1.54ms
preact-local50.69ms - 53.43msunsure 🔍
-3% - +4%
-1.54ms - +2.21ms
-

run-final

VersionAvg timevs preact-mastervs preact-local
preact-master115.30ms - 116.77ms-unsure 🔍
-1% - +1%
-0.94ms - +1.02ms
preact-local115.34ms - 116.64msunsure 🔍
-1% - +1%
-1.02ms - +0.94ms
-
03_update10th1k_x16

duration

VersionAvg timevs preact-mastervs preact-local
preact-master57.43ms - 59.60ms-unsure 🔍
-5% - +1%
-2.89ms - +0.50ms
preact-local58.41ms - 61.01msunsure 🔍
-1% - +5%
-0.50ms - +2.89ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master4.20ms - 4.20ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms
preact-local4.21ms - 4.21msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms
-
07_create10k

duration

VersionAvg timevs preact-mastervs preact-local
preact-master1231.45ms - 1242.22ms-unsure 🔍
-1% - +0%
-8.59ms - +6.16ms
preact-local1233.01ms - 1243.09msunsure 🔍
-0% - +1%
-6.16ms - +8.59ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master28.45ms - 28.57ms-unsure 🔍
-0% - +0%
-0.06ms - +0.10ms
preact-local28.43ms - 28.55msunsure 🔍
-0% - +0%
-0.10ms - +0.06ms
-
filter_list

duration

VersionAvg timevs preact-mastervs preact-local
preact-master348.07ms - 355.01ms-unsure 🔍
-0% - +3%
-0.58ms - +9.17ms
preact-local343.82ms - 350.67msunsure 🔍
-3% - +0%
-9.17ms - +0.58ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master2.14ms - 2.14ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms
preact-local2.14ms - 2.15msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms
-
hydrate1k

duration

VersionAvg timevs preact-mastervs preact-local
preact-master141.45ms - 159.20ms-unsure 🔍
-10% - +7%
-15.05ms - +10.48ms
preact-local143.43ms - 161.78msunsure 🔍
-7% - +10%
-10.48ms - +15.05ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master6.91ms - 6.92ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms
preact-local6.92ms - 6.92msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms
-
many_updates

duration

VersionAvg timevs preact-mastervs preact-local
preact-master244.29ms - 254.78ms-unsure 🔍
-2% - +5%
-3.87ms - +11.68ms
preact-local239.89ms - 251.37msunsure 🔍
-5% - +2%
-11.68ms - +3.87ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master5.77ms - 5.77ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms
preact-local5.77ms - 5.78msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms
-
text_update

duration

VersionAvg timevs preact-mastervs preact-local
preact-master56.56ms - 57.41ms-unsure 🔍
-1% - +0%
-0.85ms - +0.11ms
preact-local57.13ms - 57.57msunsure 🔍
-0% - +1%
-0.11ms - +0.85ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master1.32ms - 1.32ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms
preact-local1.32ms - 1.32msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms
-
todo

duration

VersionAvg timevs preact-mastervs preact-local
preact-master53.58ms - 56.19ms-unsure 🔍
-3% - +4%
-1.54ms - +2.19ms
preact-local53.23ms - 55.89msunsure 🔍
-4% - +3%
-2.19ms - +1.54ms
-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master1.38ms - 1.42ms-unsure 🔍
-3% - +2%
-0.04ms - +0.03ms
preact-local1.38ms - 1.43msunsure 🔍
-2% - +3%
-0.03ms - +0.04ms
-

tachometer-reporter-action v2 for Benchmarks

@github-actions

github-actions Bot commented Mar 27, 2022

Copy link
Copy Markdown

Size Change: +149 B (0%)

Total Size: 37.4 kB

Filename Size Change
dist/preact.js 4.63 kB +51 B (1%)
dist/preact.min.js 4.68 kB +49 B (1%)
dist/preact.umd.js 4.71 kB +49 B (1%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.43 kB 0 B
compat/dist/compat.umd.js 3.5 kB 0 B
debug/dist/debug.js 3.08 kB 0 B
debug/dist/debug.umd.js 3.17 kB 0 B
devtools/dist/devtools.js 232 B 0 B
devtools/dist/devtools.umd.js 316 B 0 B
hooks/dist/hooks.js 1.29 kB 0 B
hooks/dist/hooks.umd.js 1.38 kB 0 B
jsx-runtime/dist/jsxRuntime.js 342 B 0 B
jsx-runtime/dist/jsxRuntime.umd.js 425 B 0 B
server/dist/server.js 2.6 kB 0 B
server/dist/server.umd.js 2.69 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.umd.js 522 B 0 B

compressed-size-action

@coveralls

coveralls commented Mar 28, 2022

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.005%) to 99.43% when pulling 8d4eedf on queue-mounting-refs into daf90ac on main.

@JoviDeCroock JoviDeCroock deleted the queue-mounting-refs branch August 27, 2022 18:42
@andrewiggins andrewiggins added this to the v11 alpha milestone Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants