Update hyperscript "Avoid statements in view methods" docs for clarity

[elongberg]

From example Avoid statements in view methods

Constructing an virtual DOM tree procedurally can also potentially trigger expensive deoptimizations

The AVOID example proceeds to generate a list using

    var list = []
    for (var i = 0; i < vnode.attrs.items.length; i++) {
    list.push(m("li", vnode.attrs.items[i]))
    }

while the PREFER example also generates a list but uses the nicer-looking

    vnode.attrs.items.map(function(item) {
    return m("li", item)
    })

Unless I am missing something, both examples are functionally identical and have nothing to do with the quoted text. Is there any example that might relate better to the quoted text?

[leeoniya]

ironically, map is almost certainly slower because it has the overhead of a function call for each item. for/while loops with direct array access are still the fastest iteration constructs in js, as far as i know.

https://jsperf.com/native-map-versus-array-looping

[dead-claudia]

That window is starting to close, though - V8 has started creating code gen stubs for Array builtins like that, specialized for non-holey arrays.

On Mon, Sep 11, 2017, 14:12 Leon Sorokin notifications@github.com wrote:

ironically, map is almost certainly slower because it has the overhead of a function call for each item. for loops are still the fastest iteration constructs in js, as far as i know.

https://jsperf.com/native-map-versus-array-looping

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MithrilJS/docs/issues/25, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBIxNaoLG1PiqNKvDFOK4E3WkSXH_ks5shXECgaJpZM4PTg6C .

[leeoniya]

well, the same newer versions of V8 also deopt much less often, too (now handling try/catch, object key deletion, etc), putting this "to avoid deopts" advice on even shakier ground. other than cleanliness, i dont see a reason to advise against procedural code. i dont see howmap can ever be made faster than for in a single thread - equal at best.

[dead-claudia]

To be fair, for loops vs .map make very little performance difference in rendering speed because of the incredibly allocation-heavy nature of vdom and the fact redraws are throttled, so I don't find the distinction that meaningfully important. It's fast enough as-is, and converting to for loops would bloat views in a hurry due to increased code size. I'm not that worried about it because it's pretty much micro-optimizing at that point.

(When converting array-likes and non-array iterables, it's best to instead use Array.from with a callback to avoid the intermediary array allocation, but that's about it.)

---

Now, if someone wrote a JSX transform that also had iteration/conditional extensions and chose to optimize them automatically, that'd be admissible. But that whole thing is outside the scope of Mithril.

On Mon, Sep 11, 2017, 20:49 Leon Sorokin notifications@github.com wrote:

well, the same newer versions of V8 also deopt much less often, too (now handling try/catch, object key deletion, etc), putting this "to avoid deopts" advice on even shakier ground. other than cleanliness, i dont see a reason to advise against procedural code. i dont see howmap can ever be made faster than for in a single thread - equal at best.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/MithrilJS/docs/issues/25, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBKnsusSdvymI28EfQlkRCTBs_cXBks5shdUxgaJpZM4PTg6C .

[leeoniya]

I'm not that worried about it because it's pretty much micro-optimizing at that point.

right. i use map of course.

my point is probably the same as @elongberg's: the reason given for avoiding procedural code is fully merit-less, in fact, it does a disservice to users by suggesting that they will gain speed (by avoiding deopts) as a result of using the map variant - which in fact directs them to write slower code (not meaningfully slower in context, but irrefutably slower). the reason to use map is only stylistic, but this is not something that even needs to be mentioned since all the examples use this pattern. this entire AVOID/PREFER section can just be dropped to make the docs smaller.

[dead-claudia]

I'll note that the advice is to discourage mutable state in the view for ease of maintenance and the fact vnodes aren't safe to reuse, and things like if and for statements do a very good job of encouraging both cases. That's why the docs suggest that.

On Mon, Sep 11, 2017, 21:40 Leon Sorokin notifications@github.com wrote:

I'm not that worried about it because it's pretty much micro-optimizing at that point.

right. i use map of course.

my point is probably the same as @elongberg https://github.com/elongberg's: the reason given for avoiding procedural code is fully merit-less, in fact, it does a disservice to users by suggesting that they will gain speed (by avoiding deopts) as a result of using the map variant - which in fact directs them to write slower code (not meaningfully slower in context, but irrefutably slower). the reason to use map is only stylistic, but this is not something that even needs to be mentioned since all the examples use this pattern. this entire AVOID/PREFER section can just be dropped to make the docs smaller.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/MithrilJS/docs/issues/25, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBO1Sir0fxBxjG_Rz52kdNh7e2spyks5sheEHgaJpZM4PTg6C .

[leeoniya]

the advice is to discourage mutable state

then why not plainly state that rather than resorting to "deopt" FUD?

things like if and for statements do a very good job of encouraging both cases.

i disagree. unless you're talking about for-loop building, caching and reusing the child vnode array externally to render(), which is orthogonal. no idea what if encourages.

[dead-claudia]

On Mon, Sep 11, 2017, 22:36 Leon Sorokin notifications@github.com wrote:

the advice is to discourage mutable state

then why not plainly state that rather than resorting to "deopt" FUD?

I didn't write the docs on that, so feel free to write a PR removing that language.

things like if and for statements do a very good job of encouraging both

cases.

i disagree. unless you're talking about for-loop building, caching and reusing the child vnode array externally to render(), which is orthogonal. no idea what if encourages.

1. Loop building is much more verbose than using .map.
2. It's much more attractive to cache vnodes if you're using top-level if statements.
3. I'm suggesting avoiding it for general use, but there are some cases (e.g. d3 visualizations) which require it for practical reasons. Those use cases are exceedingly rare, though.

—

You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/MithrilJS/docs/issues/25, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBKWwbTtW05zBBCOpKGdrv_iDpz5Nks5she4rgaJpZM4PTg6C .

[dead-claudia]

This was fixed years ago.