Mark some 800/801s #878
Mark some 800/801s #878
Conversation
|
Late to the party ... but ...
I don't even understand what does it mean or why this is an issue. If you "shame" libraries, please be sure libraries authors understand what their libraries have been "shamed" about. All template literal based libraries look affected by their paradigm/way-to-work, so I really would like to better understand what issue we are talking about here, why is that an issue, and why this benchmark became opinionated about different paradigms per each different framework, library, API. Thanks in advance for any sort of clarification: I am really not trying to rant, I want understand, and eventually fix (as the uhtml version is centuries old too) what is this 801 about in template literals based libraries. |
|
@krausest I did some digging from the mentioned issue #801 (comment) This bit in particular:
is a bit misleading, imho, for at least the following reasons:
That's it: I don't understand in a 1K+ rows table what's the deal, as everything is already on the DOM, to use one top level listener, instead of 1K+ listeners ... how is this even thought as code smell is beyond my comprehension. I've been working on Mobile Browsers, with crappy specs in terms of RAM, since early days, and having one listener instead of 1K+ has always been a life-saver. Moreover, I can use a single event per each row, so that it'll trigger even faster (no bubbling, just capture), and all components would use exact same event all the time so that's a no-op for my non-vDOM-diffing. That's another level of cheating, accordingly to your thinking, but if I make this change I'd like to know that such change won't flag the library in the future as "huh, turned out, event delegations for these use cases do a much better job" or something. Thanks in advance for any kind of clarification around this topic. You're doing an amazing job keeping up with this project/repository, but if it ends up playing badly, in terms of libraries reputation, it'd be not super cool, imho. Have a nice rest of the day/week/end. |
|
I think this has been discussed in length and I'm afraid we won't find a solution that's considered fair by everyone. My personal interest is not to blame a framework implementation, but to classify them and I want to prevent that we end with explicit event delegation for all implementations even when it's far from an idiomatic implementation. |
|
Yeah this comes from older discussions. For a long time I was arguing this mostly alone against Virtual DOM maintainers. See #430. That issue was trying to find common ground and ultimately led to the current marking system. To be fair I was definitely in the minority so it makes sense we are where we are. Personally I don't see #801 as something worth even calling out as literally all the fast implementations do it in some form. But when I merged Lit there was a desire to mark it. I suppose it is always going to seen as a stigma while we don't mark everyone. Especially when there is a checkbox to remove it from the results. |
|
First of all, thank you both for answering. Secondly, apologies I have ... 🥁🥁🥁 ... opinions around this topic.
There's a difference between fair and possible. Libraries closer to "the metal" provide absolute freedom in terms of patterns, either delegated events or event per repeated content works. Are we sure all frameworks here are capable of such freedom and, if that's the case, why not every framework use the pattern considered "faster" in a repository that is all about benchmarks ? Basically, libraries making a conscious choice about the pattern, for this benchmark use case, are being flagged as "cheating results" ... but if that was true, why don't all other libraries use the same strategy? can they? are all libraries able to provide delegate use case? I understand this is more about "features vs DX", but I'd personally would flag instead any library incapable to perform well, in the real world, with delegates, because for an always-same tree (table's rows, ul's lists), it's a way more convenient way to handle, or even diff, a single listener. Hooks, as example, will perform very poorly with listeners re-attached per each redraw/reflow/repaint, because each listener is different each time. So why are all closer to the metal libraries, in this case where they have different ergonomics, features, possibilities, are being flagged as "not appropriate"? Moreover, every benchmark here is theoretically competing with bare metal, represented by the vanilla JS, which has the ability to do whatever it wants to beat every competitor, because that's indeed what benchmarks are about: everyone uses best approach to perform best under some specific circumstance. And here's the issue: when I've submitted my libraries, I didn't want to use the fastest possible way to perform well in here, quite the opposite. I wanted to show my libraries features, using common sense or best practice when it comes to DOM events, which is not about having thousand listeners around, just one able to control its content, like UL and TABLE do in the view itself. Now, to remove that ugly issue with note from uhtml, I need to stop showing best practices around DOM repeated content, because other frameworks decided that having a listener per each If I take this road, I can write a benchmark code that is ugly, not really underlying the library capability, and written down in a "hacky way" only because I know its internals, and I know what's the best combination of things that would make it perform faster than ever. At that point, being part of this project, which initial goal wasn't about being first, just about showing how to be fast enough with provided features, becomes less interesting, less real-world related, just an academic exercise. If this was the official goal of this project though, showing all hacks around internals, to make each library perform as fast as possible, like the vanilla base mark does, I would be OK in participating at such experiment, and change all code I know might end up in the slow path for everything we're testing here. Last, but not least, this is important to understand because I have an MR ready to land already, which makes uhtml faster (after updating it), without changing a single line of its previous code. But if it's "let's all write the same thing for everyone", then I'm back to the module with utilities every benchmark should share idea, which we previously discussed already, and happy to provide code that defeats other libraries showcases, through internal hacks/shortcuts nobody should really use out there, as these would decrease DX by far. To conclude, thanks for helping me understanding what is the real scope, goal, or purpose, of this benchmark, but I'd be disappointed if it's about discouraging patterns that just make sense in circumstances like this one: a huge table with thousand identically structured rows. |
|
P.S. most libraries are a facade of what results on the page ... pre-processor based libraries could have an easy life to consider a repeated listener per |
|
I don't think #801 should be viewed as "cheating results". I'm not sure where that impression is coming from. I would never exclude a library for #801. #430 came about because I had done similar things in Solid(#398). I made all the same arguments about it all just being DOM nodes. And how wouldn't people always just take the fastest route? And that every fast implementation does this anyway. Even made the argument this was a VDOM perspective. FYI Svelte doesn't do event delegation in this benchmark unless they added it more recently when I added Svelte 3 I made a delegated version but didn't have the blessing of the community. #561 is also worth a read. Every library can manually delegate pretty easily, I've even gone and done that in a few of my comparison to some of the most performant VDOM libraries and slightly improved their performance. Just like implicit event delegation is easy to implement in any rendering approach. It isn't limited to precompiled or VDOM.. Solid's tagged template no build version does automatic event delegations behind the scenes. This makes very little difference either way since it doesn't change the test and most of the framework authors arguing against explicit event delegation do it in their framework behind the scenes anyway. If you want to understand the counter arguments, read the piles of discussions. But I think we should focus on how to make #801 not be feel bad. Because there is nothing wrong with it (at least from my perspective if not everyone else). |
|
OK then, I guess I am adding nothing new here. The note, all notes, make one think something ain’t right out of the box, and nobody would, or should, read so much around this topic, imho, as it’s just an opinionated one. I’ll try to drop delegates and the note from my benchmark code, let’s see how that goes. I will, however, use one exact same listener for all rows, reflecting exactly what a delegate would do, but satisfying what everyone else does, ‘cause one thing are opinions, one thing is showcasing bad practices. Thanks for taking time to read, links, and discuss this. |
If I'm not mistaken, none of the current implementations violate #634 or #694 which are the only two in this list that I would truly consider "issues". So maybe just remove those and change the title to "Notes"? If a future implementation violates something and the issues need to be added back in the future perhaps add them as a separate list ("Known Issues") and use the red background that used to be there or something similar to differentiate issues from notes? |
|
The thing is I don't think #801 note should affect anyone changing their implementation. Because realistically it is the way you would handle a large table. Everyone is doing it whether manually out in the open or hidden behind the scenes. I don't think removing #801 should be done for the sake of removing the note. I personally like to see the most representative implementation. This is why I find this unfortunate. The big difference with this one compared to all the other marks is it doesn't really change what is being tested. #800 could also be done by any framework, but doing it changes what is being tested turning select row into partial update test. #772 mostly is about changing how the class on the row is set on the select test by keeping reference to DOM node and setting it directly. Admittedly #772 could really be anything. We've seen implementations use a template to initially render then literally do the same implementation as VanillaJS afterward. |
|
@ryansolid I've pushed a MR, as you can see above, and I've kept the delegate approach, as it's both the way I would go regardless of what others think, but also less memory greedy in the diffing logic. I've updated the style to showcase new syntax, and I've also provided a |
Those with errors are filtered by default (see checkbox "errors in the implementation" in "which implementations?"). They are: |
|
@krausest @ryansolid BTW, from 1more readme Delegated events Rendered DOM nodes don't have attached event listeners. Instead renderer attaches delegated event handlers to rendering root (container argument in render function) for each event type, then will use rendered app instance to discover target event handler. Events that have bubbles: true will be handled in bubble phase (from bottom to top), with proper handling of stopPropagation calls. Events that have bubbles: false (like focus event) will be handled in their capture phase on the target. This should not affect normal usage, but worth keep in mind when debugging. Note: All event handlers are active (with passive: false), and system doesn't have built-in support to handle events in capture phase. I was surprised by its performance but I believe there should be some flagging there, at least 801, but there are other levels of "cheating", even if it's, imho, a pretty amazing piece of code. |
|
P.S. for history sake, I've added some extra thoughts on the MR regarding this. uhtml could be within top 15 if it would "cheat" the select row, and I think every benchmark under 20ms there is retaining single node or avoiding any diffing around that case so performance there makes little sense, and there should be a flag for that too: "skip all rows class state" or something, to underline why inferno, uhtml, and others are 2x slower or more than vanilla. |
|
That's what I meant when I repeatedly said that most libraries delegate events. Solid, 1more, Mikado, Inferno, ivi, domvm, React. A lot of the VDOM libraries do, so calling libraries out for a best practice seems wrong. The very reason I didn't want you to remove it. But there is a subtle difference. It is implicit. There is no DOM exposure.. no end-user code crawling the tree. They attach events where you would expect on the elements and it is taken care of. It's the explicit delegation that is seen as a smell by some. This is why the note says "Manual event delegation". To my knowledge, we marked all the direct class manipulation. This was super common before and is the most common reason for marking #772. There is a slight ambiguity with #800 if the implementation uses proxies since a proxy over the DOM nodes is more or less identical. The tip-off, in that case, is if they are setting "danger" versus a boolean. With a boolean it isn't specific to the demo and at minimum that is causing a re-execution of part of the template where "danger" is. This is consistent with any reactive approach as they don't do top-down rendering. Reactive libraries work off events to avoid diffing but pay the cost in subscriptions. That is standard developer experience for them and is part of their update model and templates. This is as true for Alpine the slowest reactive library in the comparison as it is for Solid. Truthfully we can only judge on how the implementation is written not the library. It makes sense as the only real stipulation is for the implementation to be data-driven. If some internal directive caches the row that is fair as long as you aren't forcing the end-user to directly manipulate a DOM node. It's no different than how any directive works to assign an attribute. If it can be represented in the template without putting the selection state on the model data what can be said? Occasional there are hints of the library being specialized like if they expose |
|
honestly I am not sure anymore what we are benchmarking or comparing then ... it's pear vs apples across the whole table. |
|
From my point of view, this flag is not about approach itself. It's about escaping "the default way" to native DOM API calls to win benchmarks. Escaping can be done by most of the libs and will lead to performance improvements. If lib's chosen approach is not performant one, authors can take it and justify it from perspective of being "close to the metal, less prone to real-world issues" and be okay with it. Or they can make adjustments to their paradigm and purposes, and invest their time to satisfy this requirement. I'm sure that for intermediate+ devs there is no interest to see the same manual event delegation, as it is for many years, in each and every lib, but more interest to see how it can be tucked under common purpose API without making big dips in performance and without significant sacrifices in usability. Where ones see a bad limitation, other see a great challenge to solve. I can't see any value in going and mark all libs that do not use @WebReflection I can't see any github issues either here or in my repository about mentioned "levels of cheating". So, please go ahead and create it so we can discuss issues separately from current thread. Or stop throwing unreasonable BS on other devs work. I'm glad to receive your attention, but this is not kind of attention I would like to receive. |
|
@Freak613 it's unfortunate this, once respectful conversation, had to become a personal attack to me, putting words, or thoughts, I've never meant, or wrote. You say:
I wrote:
I am not English mother tongue, and maybe "cheating" wasn't the most appropriate terminology, but I've carefully quoted that word every single time, providing examples of my own libraries and explaining what "cheating" means there, to score better. By no mean I wanted to offend anyone work, if calling a library "a pretty amazing piece of code" can be even considered offensive, but the 801 flag is essentially exactly what you wrote there:
I've been advocating Web development and best practices for 20+ years now, contributed to both W3C and TC39 standards, and I'm not going to stop now. Actually, I've provided myself two versions of the same benchmark:
My documentation, when it comes to a single view that includes both top level component and inner nodes, being a tabs view, being a table, being a list element, will always show the top level delegate, specially if the list can grow exponentially. I would not propose this approach with components in isolation, as example, a single
Again, this is a best practice newcomers should learn, the sooner, the better, unless you want to be the only one "smart enough" (no offense here meant) that knows about this best practice, and apply it indeed behind the library facade. Call it a feature, that's fair enough, and that's why I've said And you perfectly underlined my initial concerns there indeed: users would think there's something wrong, or some "cheating", in these libraries flagged by 801, because that's to score better ... ad that's exactly what these libraries are not trying to do, but after all, what are benchmarks about if not something everyone points at to showcase performance of various solutions?
The long term value, since flags were introduced, is to be able to aggregate libraries by patterns. If these flags are not meant to "shame" libraries, then these could be useful to have more fair comparison. All manual DOM operations based showcases could be hidden, as example, to remove those tests based on "undocumented paths (???) or ancient knowledge about best practices from 10 years ago", but right now we can hide only by test, not by "library kind": vDOM, raw DOM, dDOM (directly diffed DOM), Components based, and so on ... implicit or explicit top level delegation, and so on. Flagging used patterns helps comparing Apple to Apple, or Features vs Features, without noise around this or that approach (see how much the entire table changes just by de-selecting the select row test, as example). As summary, I would really like to keep the tone respectful, don't put words or thoughts in other developer minds, avoid "BS" like terminology, and maybe collaborate with each other to help improving this amazing project in a way that provides even more value than it does right now, because of these hidden caveats newcomers might not understand, realize, or take a chance to learn. Thanks in advance for eventually removing the personal accusation, and I hope I've better explained my thinking around flags, or how these could be used to provide more value, and an opportunity to make that ancient knowledge available to everyone. P.S. as side note, I am really curious to know what those "undocumented paths" are about ... my Twitter DMs are open, if willing to explain what is that bit about, thanks. |
|
@WebReflection no offense, I also prefer respectful conversation, that should also be argumented, especially in the statements about somebody else work. I still can't find any clarification on real meaning of "levels of cheating" regarding 1more, not your own libs. And sorry, but for some reason I can't find any event delegation documentation or example for either hyperhtml/ligherhtml/uhtml. If it does exist, probably it's not discoverable from the projects web or github pages. Even if there is documentation and you encourage users to apply these optimizations, putting library on scale with other libs still require to setup some baselines. Trying to categorize them will be more problematic, as today libs are mixing approaches from different paradigms. And we will just move from discussions about delegation to discussions about whether some lib should be put in one category or another. Current judging based on userspace implementation seems both more universal and more clear for regular users comparing them. But it requires single common baseline. And it's under question whether categorization will help here. If you're developer interested to see how new updates affects performance, for that tests can be run locally or even integrated in CI, there is no need to publish them. If you're beginner dev trying to select lib for next project with reasonable performance, you probably won't even care about categorization, comparing just by the numbers. If you're intermediate dev who knows how these libs work internally, you also won't care about categorization because you already know about each paradigm and it's pros and cons, only being interested in how they progress and compare to each other on common scale. Trying to add more categories and flags will make benchmark results harder to navigate without providing significant value, only for someone to be label himself as "Fastest in X category". All in all, it's up to benchmark maintainer to define how its results should be read. Maybe we can vote. If you want my voice: I like this benchmark the way it is today, with common baseline, that easier to compare and with its limitations moving developers toward more automated and convenient solutions that will benefit end users. In the end we're getting more libs with nice API and good performance that do not require low-level API knowledge.
I think a lot of contributors to this repository can provide long list of best practices that they put into their libs behind facade. It doesn't make them required to be able to use these libraries. It's not about being smart enough, it's about lifetime limitation where people can not dive into each and every piece of tech to be able to craft it themselves. The ones who interested will read the sources. Others should be able proceed without getting degree in given technology. That is the point of creating facades, isn't it ?
And this will require to someone to go through all libs in order to understand type of pattern being used for each specific feature or usecase. Even if lib author can explicitly state that his lib utilizes some pattern, doesn't mean it's really is and he doesn't misunderstood something. And eventually it will become standardization of the software, with thorough specification of each feature, that will break with each new pattern or combination of patterns discovered. Still sounds too fragile for very little outcome. |
because none is needed. Everything I write tries to enable developers to write standard HTML + CSS + JS as closer to platform as possible. uhtml is hardly even a library, it's a shortcut for repeated DOM tasks, with a direct DOM differ behind the scene, that avoid taking care of updates via WeakMap, arrays, any sort of cache, as it's declarative, and it abstracts the minimum amount of operations from the platform. It's not, and it'll never be, any of my intentions to provide libraries, targeting the Web, that mislead users in what they are doing, or their intent. It's actually a goal of most of my libraries, to get Web developers closer to the platform, with tiny utilities that try to remove the most painful parts of the stack. TL;DR => MDN has plenty of articles around events delegation, and so does the rest of the Web, so these don't need to be documented in my own libraries that simply implement what the DOM does, and that might be my libraries "feature", being in full control of what's possible on the Web, compared to
Agreed, and with uhtml semantics, that view is: html`
<table>
<tbody>
${rows.map((data, id) => html`
<tr id=${id}>${data}</tr>
`)}
</tbody>
</table>
`;Now, where would you put a listener that cares only about the currently single-one selected row id? I'd put that on the table, as programming is about avoiding repeated tasks, and if the view is one only for such table, without isolated rows, also incapable of doing anything without a foreign, top level, state change they don't own, once clicked, why should I showcase the bad pattern, instead of the best practice? This looks like is the main bit we disagree about: "because nobody cares, nobody should" is not what my libraries are about, and not why I write these. If libraries/frameworks users these days are careless about the underlying logic, mechanism, stack, or technology, they build Apps against, it's one of my goal to actually help them understanding why I do things the way I do. Every abstraction is free to become the next DreamWeaver of the Web, but I've never personally bought that indirection, and my libraries goals are to enable developers to use small building blocks, while they understand, and fully control, what's going on. That is: nothing on the DOM is not possible with my libraries too, that's not always the case with higher level of abstractions. As side note: imagine #801 was commented instead as:
wouldn't you love to have such flag under your library too, as apparently it's one of the vDOM based one that took that approach? Currently, we're flagging #801 as an "issue", when all developers with knowledge know that is not an issue, that's how it should be done for this specific use case, and how real world code that scale should do too. We're basically underlying libraries for using what we all know, archaic or not, as best practice, as issue, as if using best practices should be penalized somehow, or should make developers feel comfortable about the bad practices they use, hidden behind the framework, are OK in the long term. Anyway, I hope we clarified each-other opinion about this flag. I didn't mean to offend anyone, because I believe everyone using delegate in this benchmark is doing the right thing, but I've found my showcases flagged with an issue, which triggered my comments around this unfortunate #801 flag that, imho, shouldn't exist at all, or it's not explained as "best practice", when it is. |
|
P.S. if nothing, my main complain is that #801 starts with this sentence:
The issue that labels my libraries, starts with code smell when as a matter of fact, given the benchmark structure/challenge, is a best practice, that showcases like That issue content is somehow shaming, or degrading developers effort/work, behind the scene, instead of promoting what every other framework should do, to scale real-world, and/or be faster. This is extremely annoying to me and my libraries, but few here said "801 is not shaming libraries, it's just a comment around the pattern everyone would use in the real world" ... so I'm trying to put my shoes in the user that would like to dig more in the issue itself, and the first thing they would read is that "event delegation is a code smell" ... that's not cool, imho, and that's what all my comments around #801 are about. |
|
I changed the description of #801, but I have little hope it suits everyone. At least I tried... |
|
@krausest that's perfect, than you! We can hopefully move forward now, and I am way happier with the updated description. |
I'm not stoked about this PR but fair is fair. If we are going to mark some #801 we should mark them all. I only reviewed keyed implementations and skipped ones with 694 or 772 as I figure those can be re-eval'd should they change. And of course I skipped Marionette as it's basically a 772. I can see how the argument could be made otherwise since it doesn't manipulate the DOM but creates everything with DOM APIs, but there is no "template". It's also an 801 too as the events are definitely explicitly delegated but without template syntax or whatever I don't even know how to categorize it if isn't clearly 772.