Inferno: Improve the VNode normalization process

[trueadm]

Background

Unlike React, Inferno's VNodes are not immutable. They are handled like pseudo-immutable objects in that if multiple instances are detected within Inferno, it attempts to clone the VNode. Inferno does this to reduce object creation and to improve performance (the dom property on VNodes constantly get updated as the mount/patch/unmount process happens during run-time).

What we currently do

https://github.com/trueadm/inferno/blob/master/src/core/shapes.ts#L163

Inferno currently implements a normalization process when you call createVNode. createVNode is the main entry point into creating virtual DOM for Inferno, as such, slowing down this function can have an impact on overall app performance.

Below explains what the normalization process currently does and what it deals with:

1. normalizeElement if the type property is a string, but the flags pass through says "Component", Inferno tries to correctly change this a type of "Element". This is done because JSX passes over "Component" when the JSX Element starts with an uppercase name (it assumes all uppercase names are components).

2. normalizeProps - if the props contains ref, events, children or key, usually because the VNode was created using JSX spread props, we need to put these values back onto the root of the VNode.

3. normalizeChildren - if the VNode has children, we need to do a bunch of things here (this is generally where the performance impact comes from)

   • we need to assign a property to the children if the children is an array, in this case we assign a property $ directly to the array and set it to true. We use this to know if we've seen this array before. As Inferno's VNodes are not immutable and we use them in the mount/diffing/unmounting, we need to make sure if someone has hoisted the children array, we clone it via children.splice() so we don't mutate the same object.
   • we loop through all children, detecting if we need to flatten children (where there is an array in the children), we check if there is a string or number and convert that to a VNode and if we find a child that has dom already set, we assume it's hoisted and clone it. We do this recursively for all children within children – unless the original children does not need any changes, then we simply return the original children.
   • we do not strip invalid entries from the children or assign pseudo keys to non keyed children (like React does) based on the position of the child in the array. This means that our keyed update process is extremely fragile (see #562).

Step 3 is causing some issues on performance. It feels like there must be a way to avoid doing this expensive cost if possible. Currently we can avoid normalization in benchmarks by specifying noNormalize via JSX or as a parameter of createVNode which takes away this cost.

What can we do?

We need to somehow remove normalisation. If we need to keep it, can we get away with doing it "just in time" during mounting/patching/unmounting to remove this performance loss? It's having an impact on SSR rendering where there is no DOM. It's also having an impact on animations with lots of children that have frequent updates occurring. Yes we can bypass normalization using the flag, but ideally we want to remove this altogether and have a streamlined approach. People using JSX aren't going to know how to use it properly anyway (without breaking their app).

Any ideas on how we might improve this whilst being able to still handle mixed key children and hoisting of VNodes?

@localvoid @thysultan @Havunen

[thysultan]

The holy grail would be to offload all the work to the JSX transformer, i build https://github.com/thysultan/jsx.js with this exact intention for dio. What does JSX transformer convert something like <div class="foo"></div> into?

[trueadm]

@thysultan Inferno.createVNode(4, { className: 'foo' }). How could you transform expressions that you don't know at compile time?

[thysultan]

How could you transform expressions that you don't know at compile time?

Example?

[trueadm]

@thysultan <div>{ createChildren(foo) }</div>

[thysultan]

If the function createChildren is also included in the bundle the JSX in createChildren will get transformed like every other, you would get something like.

// index.js
var foo = "Hello";
var div = <div>{ createChildren(foo) }</div>

// foo.js
function createChildren (foo) {
return <h1>foo</h1>
}

// transformed index.js
var foo = "Hello";
var div = {nodeType: 1, type: "div", props: null, children: createChildren(foo)};

// transformed foo.js
function createChildren (foo) {
    return {nodeType: 1, type: "h1", props: null, children: foo}
}

[trueadm]

@thysultan that's pretty much what the Inferno JSX plugin already does, except I stopped using inline object literals. In recent changes to V8 they don't perform as well anymore and they don't perform anywhere near as well in other JavaScript engines either.

[thysultan]

Yes the object literals was an example, optimally it would construct function calls Inferno.createVNode(4, { className: 'foo' })... why isn't the current JSX plugin doing 2, 3.2 and 3.3? I still don't understand why 3.1 is being done and adding that key to the array maybe deopting the array to a hash-map.

... so as function calls

// index.js
var foo = "Hello";
var div = <div>{ createChildren(foo) }</div>

// foo.js
function createChildren (foo) {
return <h1>foo</h1>
}

// transformed index.js
var foo = "Hello";
var div = createVNode(4, 'div', 'className', createChildren(foo), null, null);

// transformed foo.js
function createChildren (foo) {
    createVNode(?4, 'h1', null, createChildren(foo), null, null);
}

createVNode looks like it's extracting className from the object, you could do all this extraction as a compile step, i.e for key props such className, class, events, keys etc.. and pass them to the function directly, insuring the function always receives a constant amount of arguments.

[trueadm]

@thysultan it does pass them directly in – so it does what you've said above. The issue is that normalize converts text nodes to nodes, removes null etc and lots of other things.

[thysultan]

That could all get done at the compile step, at least from working on jsx.js that's what i've found.

i.e

    text: function (children) {
        return {
            nodeType: 3,
            type: 'text',
            props: emptyObject,
            children: children || '',
            _el: null
        };
    },

would constructs the internal shape for a textNode then....

    stringify: function (type, props, children, nodeType) {
         switch (nodeType) {
              // text node
              case 3: return `createVNode(3, type, props.className || null, children || '')`
         }
    }

would compile it to a function call that constructs textNodes. This all together removes the need for normalisation at runtime. You could also add an index to track the VNode's position in its children array for better keyed handling.

[trueadm]

@thysultan where would stringify get called? So if I were to take this JSX, what would the compiled code look like?

function MyComponent(props) {
    return (
        <div>
            <span>{ props.name }</span>
            <MyComponent />
            <div>
                {
                    props.children.map(child => <div>{ child }</div>)
                }
            </div>
        </div>
    )
}

[thysultan]

function MyComponent(props) {
    return (
        <div>
            <span>{ props.name }</span>
            <MyComponent />
            <div>
                {
                    props.children.map(child => <div>{ child }</div>)
                }
            </div>
        </div>
    )
}

where would stringify get called?

stringify is called when each node(including text nodes) in the AST are converted to a string,
the other methods define how the shape of a node in the AST looks like.

So if I were to take this JSX, what would the compiled code look like?

It's dependant on how you define the what strinifgy will render.

If i define the stringify method like this,

stringify: function (type, props, children, nodeType) {
    switch (nodeType) {
        // elements
        case 1: `createVNode(1, ${type}, ${props}, ${children})`; break;
        // components
        case 2: `createVNode(2, ${type}, ${props}, ${children})`; break;
        // text
        case 3: `createVNode(3, ${children})`; break;
        // potentially i can add 11 for registering fragments and 
        // 4 for svg elements if separating that from elements is optimal.
    }
}

it will produce the following code

function MyComponent (props) {
    return (
        createVNode(1, null, [
            createVNode(1, 'span', null, createVNode(3, props.name)),
            createVNode(2, MyComponent, null, null),
            createVNode(1, 'div', null, 
                props.children.map(
                    child => createVNode(1, 'div', null, createVNode(3, child))
                )
            ),
        ])
    )
}

coupled with how you define how components, elements, text and props shape the AST you could create compiled output that doesn't require normalisation.

[trueadm]

@thysultan the output you put above is pretty much what the JSX plugin already does :)

[thysultan]

That specific case is the same but I don't think it works the same way, the way i see it is all the points can be addressed in the compile step, i don't know if the current JSX plugin handles this but for example looking at just point 2. normalizeProps, you could do.

// defines how props are handled -> AST
props: function (key, value, props, node) {
     switch (key) {
         case 'ref': node.ref = value; break;
         case 'children': node.children = value; break;
         case 'className': 
         case 'class': node.className = value; break;
         case default: 
             // events
             if (key[0] === 'o' && key[1] === 'n' && key.length > 3) node.events[key] = value
             // everything else
             else { props[key] = value; }
     }
}

then the stringify function will normalize the output to avoid normalization at runtime.

[Havunen]

Hey,

How is it possible to normalise following compilation time:

class Foo extends Component {
    render() {
        return (
            <div>{getMoreNodes()}</div>
        );
    }
}

we don't know what getMoreNodes will return compile time. However can we detect such a cases? Because what we could do is skip normalisation if DOM tree doesn't have anything to normalise.

[Havunen]

@trueadm do you still have some plans how to get rid of normalizing or shall we drop nonKeyed algo because everything is now forced to be keyed unless flags state its nonKeyed but nobody is using those. We can simplify some routines by dropping nonKeyed out and save bytes

[Havunen]

Normalization process have gone through multiple optimization cycles and performance is close to optimal. However the process is now changed so that we normalize only when needed instead of normalizing always. noNormalize parameter has been removed and no normalization will be performed unless told. createElement and hyperscript API's will still do normalization and babel plugin will intelligently find situations when normalization is needed. FE: Any JSX expression will cause children to be normalized, using spread operator in JSX element causes normalizeProps to be called. SSR will never normalize. Also defaultProps has been moved out of normalization as that is core functionality and not related to normalization.

SSR is showing 10 times better runtime performance in isomorphic uibenchmark. It is no longer bottlenecked by normalization loop.
Client side performance is showing 0 - 33% improvement based on use case. With many static elements performance is increased about 33%, but in scenarios which has many JSX Expressions there is no change.

I'm closing this task.
Related commit: https://github.com/infernojs/inferno/commit/f51ade5fe3864beba91e68171d241908a767422c

This will ship in Inferno v4

[leeoniya]

so, is the createVNode api the only way to fulfill the promise of the benchmark results?

[Havunen]

@leeoniya currently yes, createElement has reserved all extra parameters for rest spread children so it cannot be extended, for hyperscript API we could provide third parameter noNormalize which would stop it doing normalization for children. It could be used when user has taken care of nulls/keys at application level. Do you think that would be useful?

[leeoniya]

Do you think that would be useful?

yes. no one can reasonably be expected to use createVNode, so the bench results are a pipe dream. adding it to hyperscript at least makes it palatable for DX. i'm considering adding the same to domvm's normalization passes [1].

[1] https://github.com/leeoniya/domvm/blob/3.x-dev/src/view/preProc.js

[Havunen]

so, is the createVNode api the only way to fulfill the promise of the benchmark results?

JSX users will get this benefit out of box. As we can do processing compile time.

[Havunen]

I added noNormalize parameter to inferno-hyperscript API.

[leeoniya]

ah ok. i misread the original comment. it would make sense to rewrite the benchmark impls in JSX then. using a non-standard apis to get the numbers has a certain smell to it :)

[Havunen]

Yeah, but I dont know is it "non-standard", it has been the Inferno way since 0.6? createVNode is lower level API to create virtual nodes while createElement and hyperscript are higher level API's.

[leeoniya]

perf numbers are disingenuous if the way to achieve "Insane Performance" is to rewrite whole apps in a way that's neither "React Compatible" nor promoted by the docs.

i suppose everyone has their threshold for how much impl tweaking is acceptable before it becomes uncomfortably misleading. i think adding a hyperscript param is a great optimization escape hatch, since it just slightly tweaks the existing templates but having to rewrite using an entirely DX-hostile createVNode crosses what i would consider a reasonable expectation.

i think it's great news that these opimizations are now available automatically via a DX-friendly JSX or with slight tweaks to hyperscript. it would be unfortunate if the bench examples continue to promote the createVNode apis when they're not actually necessary.

it has been the Inferno way since 0.6

and the perf claims have been misleading for as long, if not longer, IMO. just my $0.02, or maybe $0.04 since i participate pretty actively in this space.

[Havunen]

Yep, I fully agree with you. In the next major version (4) we are trying to bring these performance gains to normal applications to people who don't know and does not need to know any implementation details. Moving normalization detection to compilation process is one step forward that goal.

[brodybits]

perf numbers are disingenuous if the way to achieve "Insane Performance" is to rewrite whole apps
[...]

I would not agree 100%. I discovered https://github.com/krausest/js-framework-benchmark from https://github.com/hyperapp/hyperapp/issues/13 and https://github.com/picodom/picodom/issues/52 which is not API dependant. Would it make any sense to consider benchmarking and documenting createVNote as its own component? Or even consider reusing something like picodom?

[leeoniya]

which is not API dependant

can you clarify what you mean by this?

[brodybits]

which is not API dependant

can you clarify what you mean by this?

krausest / js-framework-benchmark does benchmark measurements of a number of operations on each framework, regardless of API. From https://github.com/krausest/js-framework-benchmark#about-the-benchmarks:

The following operations are benchmarked for each framework: * create rows: Duration for creating 1000 rows after the page loaded. * replace all rows: Duration for updating all 1000 rows of the table (with 5 warmup iterations). * partial update: Time to update the text of every 10th row for a table with 10000 rows (with 5 warmup iterations). * select row: Duration to highlight a row in response to a click on the row. (with 5 warmup iterations). * swap rows: Time to swap 2 rows on a 1K table. (with 5 warmup iterations). * remove row: Duration to remove a row. (with 5 warmup iterations). * create many rows: Duration to create 10,000 rows * append rows to large table: Duration for adding 1000 rows on a table of 10,000 rows. * clear rows: Duration to clear the table filled with 10.000 rows. * clear rows a 2nd time: Time to clear the table filled with 10.000 rows. But warmed up with only one iteration. * ready memory: Memory usage after page load. * run memory: Memory usage after adding 1000 rows. * startup time: Duration for loading and parsing the javascript code and rendering the page.

krausest / js-framework-benchmark gives us a comparison of how long each of these operations would take on a number of diverse frameworks such as Angular 5, Angular 1, picodom, dio, Inferno, React, Vue, Preact, etc. I think it would also be nice to add a benchmark for using createVNode by itself. Snapshot of the krausest / js-framework-benchmark results is available at: https://rawgit.com/krausest/js-framework-benchmark/master/webdriver-ts-results/table.html

Incidentally Inferno seems to do well on krausest / js-framework-benchmark, the one small thing may be "clear rows".

[Havunen]

@brodybits its already doing noNormalize so its calling createVNode behind the scenes
(https://github.com/krausest/js-framework-benchmark/blob/master/inferno-v3.10.1-keyed/src/controller.jsx#L41-L48), my change here is that everybody will get close to the same performance without source code changes. + Server side rendering gets 10 times faster

The performance might improve slightly in v4 but no major gains are expected in that benchmark.

[leeoniya]

@brodybits if you look at either of the threads you linked, you'll see that i contribute regularly to this bench :)

the inferno impl there is good. it uses JSX w/noNormalize: https://github.com/krausest/js-framework-benchmark/blob/master/inferno-v3.10.1-keyed/src/controller.jsx

[brodybits]

Thanks @Havunen @leeoniya for the clarifications, definitely looks like awesome work. Will Inferno link to krausest / js-framework-benchmark?

[trueadm]

Great work @Havunen, this is a great enhancement for Inferno. I'm looking into similar things for React right now. I'll also be sharing a blog post soon about the work that has gone into thew new React compiler that I'm building into Prepack and what we're looking to experiment with next year.