glimmerjs / glimmer-vm

MIT License
1.13k stars 191 forks source link

[memory] replace forEach with arrayFrom #1522

Closed lifeart closed 11 months ago

lifeart commented 11 months ago

Spillover PR from https://github.com/glimmerjs/glimmer-vm/pull/1515#discussion_r1418465619

Left: Original Right: This change

image image

It seems we could have 10% memory usage improvement for large data sets.

lifeart commented 11 months ago

@NullVoxPopuli I can't say because JS framework benchmark seems support only chrome.

But, using Array.from is definitely less hacky comparing to existing implementation (?micro optimisation?) and should win over years.

I think forEach here is used from IE11 support times.

image
NullVoxPopuli commented 11 months ago

makes sense to me! thanks!