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

Improve the VNode normalization process #565

Closed
trueadm opened this Issue Dec 12, 2016 · 32 comments

Comments

Projects
None yet
6 participants
@trueadm
Collaborator

trueadm commented Dec 12, 2016

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

@Havunen Havunen added the enhancement label Dec 12, 2016

@thysultan

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator

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?

Collaborator

thysultan commented Dec 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 13, 2016

Collaborator

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

Collaborator

trueadm commented Dec 13, 2016

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

@thysultan

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator

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

Example?

Collaborator

thysultan commented Dec 13, 2016

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

Example?

@trueadm

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 13, 2016

Collaborator

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

Collaborator

trueadm commented Dec 13, 2016

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

@thysultan

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator

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}
}
Collaborator

thysultan commented Dec 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 13, 2016

Collaborator

@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.

Collaborator

trueadm commented Dec 13, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator

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.

Collaborator

thysultan commented Dec 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 13, 2016

Collaborator

@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.

Collaborator

trueadm commented Dec 13, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator

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.

Collaborator

thysultan commented Dec 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 13, 2016

Collaborator

@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>
	)
}
Collaborator

trueadm commented Dec 13, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator
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.

Collaborator

thysultan commented Dec 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 13, 2016

Collaborator

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

Collaborator

trueadm commented Dec 13, 2016

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

@thysultan

This comment has been minimized.

Show comment
Hide comment
@thysultan

thysultan Dec 13, 2016

Collaborator

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.

Collaborator

thysultan commented Dec 13, 2016

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.

@trueadm trueadm self-assigned this Dec 19, 2016

@trueadm trueadm added this to the Future milestone Dec 19, 2016

@Havunen

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Jan 3, 2017

Member

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.

Member

Havunen commented Jan 3, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Mar 4, 2017

Member

@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

Member

Havunen commented Mar 4, 2017

@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 Havunen added the v4 label May 20, 2017

@LukeSheard LukeSheard removed the v5 label Jun 10, 2017

@Havunen

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member

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: f51ade5

This will ship in Inferno v4

Member

Havunen commented Dec 15, 2017

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: f51ade5

This will ship in Inferno v4

@Havunen Havunen closed this Dec 15, 2017

@leeoniya

This comment has been minimized.

Show comment
Hide comment
@leeoniya

leeoniya Dec 15, 2017

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

leeoniya commented Dec 15, 2017

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

@Havunen

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member

@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?

Member

Havunen commented Dec 15, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@leeoniya

leeoniya Dec 15, 2017

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

leeoniya commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member
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.

Member

Havunen commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member

I added noNormalize parameter to inferno-hyperscript API.

Member

Havunen commented Dec 15, 2017

I added noNormalize parameter to inferno-hyperscript API.

@leeoniya

This comment has been minimized.

Show comment
Hide comment
@leeoniya

leeoniya Dec 15, 2017

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 :)

leeoniya commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member

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.

Member

Havunen commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@leeoniya

leeoniya Dec 15, 2017

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.

leeoniya commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member

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.

Member

Havunen commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@brodybits

brodybits Dec 15, 2017

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 jorgebucaran/hyperapp#13 and jorgebucaran/superfine#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?

brodybits commented Dec 15, 2017

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 jorgebucaran/hyperapp#13 and jorgebucaran/superfine#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

This comment has been minimized.

Show comment
Hide comment
@leeoniya

leeoniya Dec 15, 2017

which is not API dependant

can you clarify what you mean by this?

leeoniya commented Dec 15, 2017

which is not API dependant

can you clarify what you mean by this?

@brodybits

This comment has been minimized.

Show comment
Hide comment
@brodybits

brodybits Dec 15, 2017

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".

brodybits commented Dec 15, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 15, 2017

Member

@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.

Member

Havunen commented Dec 15, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@leeoniya

leeoniya Dec 15, 2017

@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

leeoniya commented Dec 15, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@brodybits

brodybits Dec 15, 2017

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

brodybits commented Dec 15, 2017

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

@trueadm

This comment has been minimized.

Show comment
Hide comment
@trueadm

trueadm Dec 15, 2017

Collaborator

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.

Collaborator

trueadm commented Dec 15, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment