sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
79.14k stars 4.16k forks source link

Update JS Frameworks Benchmark #2919

Closed ryansolid closed 5 years ago

ryansolid commented 5 years ago

I've gone ahead and updated Svelte to 3.2.0 from the 2.x version. But when the results came up not as impressive as hoped I dug in some more and all of the top implementations use Event Delegation which I have now added in this pull request: https://github.com/krausest/js-framework-benchmark/pull/561

However, since some of the implementors (especially on Virtual DOM side of the camp) think that explicit event delegation doesn't belong in the benchmark(while their libraries are doing it implicitly) it's been a blocked somewhat. But understand that a good number of the implementations use explicit(including lit-html etc..) so it isn't unprecedented. However, this has devolved into a debate about what best represents Svelte, the best performance, or the least code. A position I can't authoritatively make a call on.

What we likely need is @Rich-Harris or some recognized contributor from Svelte to weigh in and push this through. If anyone has time it might worth doing as it's a good showcase of performance, and in current state Svelte doesn't look much different than Preact. I figured it would be of interest since this is one of the more popular benchmarks. But if not feel free to close this issue, and I will withdraw my pull request.

btakita commented 5 years ago

Would it be possible to delegate event handling to the root svelte component in the component tree?

ryansolid commented 5 years ago

@btakita Not quite sure if it is my misunderstanding of the suggestion. But in this benchmark there is only one Svelte Component. And really this just tests the efficiency of the each helper. So event delegation is fairly straight forward in that any element above the each helper suffices and the label in the row can provide id to lookup the corresponding data context. The extra code is basically facilitating this lookup which we'd have to do anyway.

The challenge I'm facing is more political. Every top library in that benchmark uses event delegation of some sort. There is a fairly large number of contributors who feel that event delegation that isn't implicitly part of library doesn't belong there. However, every top library that that doesn't do implicitly does so explicitly. This includes lit-html, lighterhtml, surplus, and pretty much every non-Virtual DOM library in the top performance tier.

Since this is seen as a dirty stigma no one is going to take me as representing Svelte so this will sit still likely until @Rich-Harris or the like comes in and goes "That looks like good idiomatic Svelte, I think it is a good representation of library for this benchmark". Justin Fagnani essentially had to do the same for lit-html and Andrea Giammarchi for lighterhtml. To me this is complete nonsense but when seeking consensus on this in the past we haven't reached agreement. Stefan won't block any submission that represents the library as the authors intended, but he will be hesitant to accept anything where there is controversy and no clear authority.

mrkishi commented 5 years ago

@ryansolid I believe btakita's question was whether Svelte could add event handling delegation to the framework — something that doesn't seem too hard on first look. I'm not sure if implicit delegation would be in line with Svelte's other features, though. IMO, an event modifier is more along the lines, if I may say so:

<table class="table table-hover table-striped test-data">
    <tbody>
        {#each data as row (row)}
            <tr class:danger={selected === row}>
                <td class="col-md-1">{row.id}</td>
                <td class="col-md-4">
                    <a on:click|delegate={() => selected = row}>{row.label}</a>
                </td>
                <td class="col-md-1"><a on:click|delegate={() => remove(row)}><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
                <td class="col-md-6"></td>
            </tr>
        {/each}
    </tbody>
</table>

However, I have the impression they would bark at the most straightforward way to implement this in Svelte: linking the required context data to those nodes directly. Not that I think this should be taken into account when figuring this out...

Just out of curiosity, how do VDOM libraries implement implicit delegation? I assume most have to match actual nodes to vnodes/listeners somehow for wiring events?

evs-chris commented 5 years ago

Having implemented implicit event delegation in a vdom-ish library, I would caution that there are a quite a few corner cases hiding in the weeds waiting to bite the unwary. IMO explicit delegation in a helper-y way would be more of a svelte approach. I'd probably invert that last example and let the delegation root declare itself as such and have that propagate down to the leaves.

FWIW last I tried, I could reduce render time for the 10k render benchmark by 5-7% using delegation. Svelte may benefit more since it has way less runtime bookkeeping.

ryansolid commented 5 years ago

Yeah in this case the pull request actual already has the difference in results posted by the benchmark author here. It's considerable. More than I was expecting.

Implicit delegation is a bit more expensive. Generally the event handler registration stuff on document.body is fairly straightforward, generalizing the data lookup is another thing. Some libraries don't both and just use the closure over the function but that still requires a function per line. Smarter approaches associate the data with the node and work off a single handler. Inferno for instance uses a linkEvent helper to bind it. You can do this with real DOM nodes too not just virtual DOM nodes. I've implemented this in non-Virtual DOM libraries. The compiler could handle this fairly easily with some indicating syntax like posted above.

That being said it's sort of questionable. I mean there are other complications. One for instance is Shadow DOM retargetting and composable events. The way the Virtual DOM libraries do event delegation is completely broken there. It is possible to solve basically using composedPath to reverse the retargetting and using partially synthetic events (adding your own properties on top) but it starts getting pretty complicated to do with small code base.. Other interesting things to consider around timing of cancellation and combination of delegated and non-delegated events. I've implemented this before but it seems a lot for Svelte to do if bundle size is the priority.

So it's not so much that non-VDOM libraries cannot do implicit event delegation. It's more that they are less incentivized to synthetic events which usually go hand in hand. Knowing that event.target is a DOM node isn't heresy. VDOM tries so hard to abstract at a certain point though it feels like that is a leak in that abstraction. I get it. Especially if you start traversing those nodes in the open or reading properties from them, it's a completely different tree than the virtual tree. But I mean how much do you care in Svelte?