google / incremental-dom

An in-place DOM diffing library
http://google.github.io/incremental-dom/
Apache License 2.0
3.54k stars 180 forks source link

Lazy load NodeData properties #334

Closed jridgewell closed 6 years ago

jridgewell commented 7 years ago

A few things:

jridgewell commented 7 years ago
Test Old New
List Selection 0.262ms 0.246ms
List Creation 1.839ms 1.757ms
Mutation Creation 1.770ms 1.692ms
Mutation High 0.731ms 0.705ms
jridgewell commented 6 years ago

Ping @sparhami: rebased this off latest updates

sparhami commented 6 years ago

I remember looking at this a while ago and couldn't figure out if there was any benefit or not. I tried doing a heap dump that indicated this didn't really help with heap usage even when you have elements without any attributes.

One idea I had is whether or not it makes a difference to allocate the attributes array with the correct size to begin with. Ideally, that should avoid:

a. Reallocation for the array as we loop through the attributes and add them to the array b. Extra empty space in the array (usually array allocation adds extra empty space to avoid reallocating on every item added)

I don't really have an understanding of how arrays work in v8 or other JS engines, but it seems that allocating with the right size to begin with at least shouldn't hurt. I found this JS perf that seems to indicate that allocating with the right size might be useful: https://jsperf.com/preallocating-array/8

It seems with this change, now that we are allocating when we actually have the arrays in question, we can create the array with the right initial size.

jridgewell commented 6 years ago

Let me split this into two PRs.

One idea I had is whether or not it makes a difference to allocate the attributes array with the correct size to begin with

This has the potential for optimization, and it's common in things like lodash and babel. But, it has downsides in V8: it actually creates a holey array. This makes basic iteration slower, because it has to do more checks to see if the array has an element at the i index instead of just getting the element in a packed array.

We could do the preallocation, then slice the holey array to get a properly sized packed array. But that's an extra allocation, too. Or, use an shared attrsBuilder and slice it to get the packed array when necessary.

jridgewell commented 6 years ago

For reference: the changes necessary for preallocation of the attrsArr.