Skip to content

chore: improve old value implementation performance #15854

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

Closed
wants to merge 4 commits into from

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Apr 30, 2025

This improves the signal runtime performance by opting for a linked list rather than a Map for storing old values. In theory a Map should be O(1) for lookups, but they come with overhead in terms when both setting a value and when clearing the Map down. Instead, using a linked list avoids this overhead except in the case where there might be many signal changes in the same update – in which case, maybe we should still opt to use a Map for those cases. Here are the benchmark results:

Main:

{
  benchmark: 'sbench_create_signals',
  time: '401.26',
  gc_time: '29.60'
}
{ benchmark: 'sbench_create_0to1', time: '8.17', gc_time: '0.00' }
{ benchmark: 'sbench_create_1to1', time: '29.06', gc_time: '0.59' }
{ benchmark: 'sbench_create_2to1', time: '27.08', gc_time: '0.52' }
{ benchmark: 'sbench_create_4to1', time: '25.51', gc_time: '0.61' }
{ benchmark: 'sbench_create_1000to1', time: '24.78', gc_time: '0.49' }
{ benchmark: 'sbench_create_1to2', time: '12.27', gc_time: '0.00' }
{ benchmark: 'sbench_create_1to4', time: '16.14', gc_time: '1.18' }
{ benchmark: 'sbench_create_1to8', time: '13.43', gc_time: '1.21' }
{ benchmark: 'sbench_create_1to1000', time: '4.89', gc_time: '0.00' }
{
  benchmark: 'kairo_avoidable_owned',
  time: '415.52',
  gc_time: '77.30'
}
{
  benchmark: 'kairo_avoidable_unowned',
  time: '507.39',
  gc_time: '73.81'
}
{ benchmark: 'kairo_broad_owned', time: '414.50', gc_time: '2.02' }
{ benchmark: 'kairo_broad_unowned', time: '413.50', gc_time: '2.16' }
{ benchmark: 'kairo_deep_owned', time: '181.87', gc_time: '1.93' }
{ benchmark: 'kairo_deep_unowned', time: '806.19', gc_time: '4.01' }
{ benchmark: 'kairo_diamond_owned', time: '340.28', gc_time: '34.45' }
{
  benchmark: 'kairo_diamond_unowned',
  time: '434.26',
  gc_time: '34.75'
}
{ benchmark: 'kairo_triangle_owned', time: '96.85', gc_time: '4.79' }
{
  benchmark: 'kairo_triangle_unowned',
  time: '198.79',
  gc_time: '4.57'
}
{ benchmark: 'kairo_mux_owned', time: '298.60', gc_time: '3.77' }
{ benchmark: 'kairo_mux_unowned', time: '450.16', gc_time: '3.95' }
{ benchmark: 'kairo_repeated_owned', time: '54.82', gc_time: '5.33' }
{ benchmark: 'kairo_repeated_unowned', time: '55.61', gc_time: '5.08' }
{ benchmark: 'kairo_unstable_owned', time: '88.33', gc_time: '5.46' }
{
  benchmark: 'kairo_unstable_unowned',
  time: '103.47',
  gc_time: '4.81'
}
{ benchmark: 'mol_bench_owned', time: '231.41', gc_time: '0.75' }
{ benchmark: 'mol_bench_unowned', time: '248.76', gc_time: '1.18' }

Finished reactivity benchmarks.

{ suite_time: '5902.90', suite_gc_time: '304.32' }

This PR:

{
  benchmark: 'sbench_create_signals',
  time: '296.43',
  gc_time: '78.94'
}
{ benchmark: 'sbench_create_0to1', time: '7.96', gc_time: '0.00' }
{ benchmark: 'sbench_create_1to1', time: '19.28', gc_time: '1.72' }
{ benchmark: 'sbench_create_2to1', time: '17.12', gc_time: '1.84' }
{ benchmark: 'sbench_create_4to1', time: '15.60', gc_time: '1.78' }
{ benchmark: 'sbench_create_1000to1', time: '14.50', gc_time: '1.63' }
{ benchmark: 'sbench_create_1to2', time: '9.62', gc_time: '0.53' }
{ benchmark: 'sbench_create_1to4', time: '19.26', gc_time: '6.70' }
{ benchmark: 'sbench_create_1to8', time: '14.69', gc_time: '3.57' }
{ benchmark: 'sbench_create_1to1000', time: '4.36', gc_time: '0.00' }
{
  benchmark: 'kairo_avoidable_owned',
  time: '359.36',
  gc_time: '67.77'
}
{
  benchmark: 'kairo_avoidable_unowned',
  time: '450.11',
  gc_time: '67.10'
}
{ benchmark: 'kairo_broad_owned', time: '416.18', gc_time: '1.68' }
{ benchmark: 'kairo_broad_unowned', time: '414.42', gc_time: '2.45' }
{ benchmark: 'kairo_deep_owned', time: '178.57', gc_time: '1.29' }
{ benchmark: 'kairo_deep_unowned', time: '823.22', gc_time: '3.19' }
{ benchmark: 'kairo_diamond_owned', time: '310.31', gc_time: '31.70' }
{
  benchmark: 'kairo_diamond_unowned',
  time: '406.96',
  gc_time: '31.66'
}
{ benchmark: 'kairo_triangle_owned', time: '93.90', gc_time: '5.69' }
{
  benchmark: 'kairo_triangle_unowned',
  time: '199.99',
  gc_time: '6.23'
}
{ benchmark: 'kairo_mux_owned', time: '299.27', gc_time: '3.63' }
{ benchmark: 'kairo_mux_unowned', time: '451.08', gc_time: '3.53' }
{ benchmark: 'kairo_repeated_owned', time: '51.61', gc_time: '4.41' }
{ benchmark: 'kairo_repeated_unowned', time: '52.58', gc_time: '4.54' }
{ benchmark: 'kairo_unstable_owned', time: '83.56', gc_time: '5.82' }
{
  benchmark: 'kairo_unstable_unowned',
  time: '101.41',
  gc_time: '5.96'
}
{ benchmark: 'mol_bench_owned', time: '232.01', gc_time: '0.80' }
{ benchmark: 'mol_bench_unowned', time: '249.05', gc_time: '1.24' }

Finished reactivity benchmarks.

{ suite_time: '5592.41', suite_gc_time: '345.40' }

Copy link

changeset-bot bot commented Apr 30, 2025

🦋 Changeset detected

Latest commit: cf263ab

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@15854

@Rich-Harris
Copy link
Member

Visual benchmark comparison:

sbench_create_signals
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 437.74ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 438.42ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 194.30ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 195.91ms
sbench_create_0to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 5.04ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  4.88ms
sbench_create_1to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 22.14ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 21.80ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  2.74ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.82ms
sbench_create_2to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 19.53ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 19.47ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.76ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.70ms
sbench_create_4to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 18.24ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 17.99ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.68ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.69ms
sbench_create_1000to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 16.84ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 16.67ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.72ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.73ms
sbench_create_1to2
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 10.43ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 10.39ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.65ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.66ms
sbench_create_1to4
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 8.59ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  8.09ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.57ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   0.50ms
sbench_create_1to8
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 6.82ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   6.18ms
sbench_create_1to1000
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.58ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.60ms
kairo_avoidable_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   410.71ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 463.20ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   99.12ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 112.87ms
kairo_avoidable_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   508.87ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 559.25ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  103.44ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 111.25ms
kairo_broad_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 436.59ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 438.06ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   2.65ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 3.00ms
kairo_broad_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 435.41ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 437.94ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    2.73ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 3.13ms
kairo_deep_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 191.62ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 194.24ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.45ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.49ms
kairo_deep_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 927.49ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 949.53ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼       2.68ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 3.88ms
kairo_diamond_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  351.37ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 368.12ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  53.44ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 55.74ms
kairo_diamond_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  454.90ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 476.06ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   53.03ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 57.60ms
kairo_triangle_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  100.44ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 107.59ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    6.92ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 8.34ms
kairo_triangle_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  223.84ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 231.52ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   8.78ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 9.90ms
kairo_mux_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  317.05ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 326.00ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  5.11ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 5.30ms
kairo_mux_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 486.09ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 486.68ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  5.25ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 5.53ms
kairo_repeated_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  54.33ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 58.36ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    6.50ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 7.67ms
kairo_repeated_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  55.60ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 59.83ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼     6.57ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 7.98ms
kairo_unstable_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  87.60ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 92.33ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 8.44ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  8.20ms
kairo_unstable_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  105.36ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 108.36ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 8.54ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼     6.83ms
mol_bench_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 281.82ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 284.48ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.49ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼       1.77ms
mol_bench_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 298.66ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 300.02ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.52ms
    b: ◼◼◼◼◼◼◼◼             1.04ms

While this PR is faster on some benchmarks, it's slower on others. It seems like using a linked list should be very slightly faster in the common case but significantly slower in the pathological case, and since we're talking about microseconds in the common case it might be preferable to keep the existing implementation, especially since it's much simpler? Or is the current implementation causing real-world performance issues?

@trueadm
Copy link
Contributor Author

trueadm commented May 13, 2025

Was that with the other PR merged too? It's odd because locally I get different results than you do, but you can still see that the kairo benchmarks are benefited. I wonder if we can improve on this PR though by:

  • maybe switching to a Map if the size hits a certain figure?
  • maybe keeping the Map only but avoiding assigning to it if there's no teardown used

@svelte-docs-bot
Copy link

@Rich-Harris
Copy link
Member

It was with the other PR merged, yeah. Just brought everything up to date and did another run with similar results:

updated results
sbench_create_signals
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   434.54ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 476.38ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    191.17ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 226.08ms
sbench_create_0to1
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.86ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.87ms
sbench_create_1to1
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 21.85ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 22.02ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  2.74ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.86ms
sbench_create_2to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 19.34ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 19.26ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.71ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.76ms
sbench_create_4to1
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 18.17ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 18.23ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  2.65ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.72ms
sbench_create_1000to1
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 16.85ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  16.40ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.73ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.67ms
sbench_create_1to2
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 10.51ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 10.37ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    0.58ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.67ms
sbench_create_1to4
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  8.16ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 8.54ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  0.46ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.49ms
sbench_create_1to8
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  6.23ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 6.69ms
sbench_create_1to1000
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.58ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.60ms
kairo_avoidable_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    250.47ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 292.20ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼              2.53ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 7.33ms
kairo_avoidable_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   357.32ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 400.02ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼            3.29ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 7.54ms
kairo_broad_owned
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 440.42ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 430.76ms
kairo_broad_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 428.47ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 430.55ms
kairo_deep_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 184.97ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 188.55ms
kairo_deep_unowned
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 924.93ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 924.54ms
kairo_diamond_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  278.39ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 291.31ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼        2.55ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.04ms
kairo_diamond_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  384.60ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 400.26ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼        2.64ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 4.18ms
kairo_triangle_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  91.92ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 95.20ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.72ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   0.65ms
kairo_triangle_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 213.40ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 214.65ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.77ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   0.68ms
kairo_mux_owned
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 320.28ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 316.63ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.13ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.12ms
kairo_mux_unowned
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 478.68ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 478.09ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼   2.00ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 2.20ms
kairo_repeated_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  45.07ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 47.36ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.53ms
    b: ◼◼◼◼◼◼◼◼◼◼           0.26ms
kairo_repeated_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  46.07ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 47.26ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.42ms
    b: ◼◼◼◼◼◼◼◼◼◼           0.22ms
kairo_unstable_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  75.53ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 80.00ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  0.73ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.77ms
kairo_unstable_unowned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  95.37ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 98.52ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼  0.84ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.89ms
mol_bench_owned
  time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 282.59ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 283.01ms
  gc_time: fastest is a (optimize-old-values)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼    0.27ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.31ms
mol_bench_unowned
  time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 299.29ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 296.21ms
  gc_time: fastest is b (main)
    a: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼ 0.32ms
    b: ◼◼◼◼◼◼◼◼◼◼◼◼◼◼◼      0.24ms

So yeah, gc_time is faster on the kairo benchmarks but since gc_time for these tests is such a miniscule percentage of time + gc_time, it seems like the real world impact would be negligible. And I suspect the benchmarks just aren't covering the case where this approach turns out to be slower because we get caught in the while loop (where we'd presumably go from O(n) to O(n^2)?), so for me it comes back to whether this is actually causing problems or if it's more about scoring higher on the benchmark, particularly given the added complexity.

Switching to a Map at a certain threshold feels like a bad choice — we can't really know what the appropriate threshold is across different machines/engines, and it adds even more complexity. As for 'if there's no teardown used' we would always hit false negatives, because the existence of effect.teardown somewhere in the graph doesn't tell us whether values are read in those teardowns or which ones. So again I'm not 100% sure it's worth it.

@trueadm
Copy link
Contributor Author

trueadm commented May 13, 2025

Let's leave this optimisation out of it then :) thanks for digging into it and reviewing though.

@trueadm trueadm closed this May 13, 2025
@dummdidumm dummdidumm deleted the optimize-old-values branch May 13, 2025 14:20
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.

2 participants