krausest / js-framework-benchmark

A comparison of the performance of a few popular javascript frameworks
https://krausest.github.io/js-framework-benchmark/
Apache License 2.0
6.56k stars 811 forks source link

Proposal: suggest a common utility to "rule them all" #681

Closed WebReflection closed 4 years ago

WebReflection commented 4 years ago

AS I've found myself copying and pasting a similar domc scope utility all over, I wonder if we could simply normalize all tests (which is also an opportunity to call for updates, so that those that won't be updated can be discarded) through an utility that can easily handle all cases, including returning {...scope} whenever it's needed to trigger updates (i.e. neverland, or react-hooks).

For this purpose, I've created the js-framework-benchmark-utils module, which, accordingly with the discussion and the outcome we'll have in this ticket (if any), could be used already for all my libraries.

This should make any benchmark scope/state handling at pair, in terms of performance, so that nobody can cheat or do things wrong, simplifying both maintenance of every library/framework in this repository, but also helping in making future pull requests easier to review.

Happy to hear your opinion 👋

P.S. if it's OK to use such module, I could update all my PRs with it, so that you won't need to re-run tests after. Please let me know, thanks.

ryansolid commented 4 years ago

This approach could work for about 70% of the libraries I figure. Any based on top-down reconciliation. It doesn't work well for Reactive libraries though. There is no update function for them and they rely on specialized data objects. This is true for even the ones that use things like Proxies to mask their Reactive data.

If you are not familiar with what I mean, I'm talking about libraries like KnockoutJS. Surplus, Solid, Sinuous, Fidan, Ko-JSX, MobX-JSX are of this type (and probably a few others Ember worked this way in the past, Ractive, Svelte). These libraries even if using Proxies often rely on mutation rather than immutable cloning. This is true of even Mikado (proxy). So all the destructuring won't work with them. I suspect even Svelte would also have a few gotchas besides not having the update function. In many cases even manufacturing an update function would be a performance hit since you'd be forcing them to diff more changes since the granularity is a single dataset. These libraries work off finer-grained changes. What makes JS Framework Benchmark unique is that it lets you bring your own data management. There is no need to force this into a data snapshot benchmark for libraries that don't work that way.

Looking at your utils implementations one is not like the other. I think add may not universally work since it mutates rather returns a new array. It probably should use concat for a number of immutable data based libraries like most virtual dom libraries. It is the only one in there that doesn't create a new array. Not all of them are optimized for deep reconciliation like domc or imba which are actually a 3rd approach to rendering.

I've thought about this in the past since there are for the most part there are 3 types of rendering there might be 3 general templates here. The challenge is that on the reactive side the data is so specialized even if we came up with a common pattern it would not be universal. Still with that one change to add I think it could be useful to a large number of libraries so I think this can have value regardless for the majority of libraries trying to make an implementation.

WebReflection commented 4 years ago

There is no update function for them and they rely on specialized data objects.

The update is mostly optional, and it simply receives the very same "scope" (or state) object returned by the function itself.

const state = Scope(Object);
state.select(id);
state.update();
// and all others, including
state.data;

This should be sufficient to even use/update redux or other libraries, keeping the data generation itself, and data creation, swap, update, select, remove performance identical ... wouldn't this be enough?

It probably should use concat for a number of immutable data based libraries like most virtual dom libraries.

That adds unnecessary extra memory to the table, but the update could be used to invalidate mutation, as in:

const state = Scope(scope => {
  scope.data = scope.data.slice(0);
  // now call/use/invoke whatewer consumes data
});

The concat also doesn't mutate the items already there (and it shouldn't, imho), so the fact there is a new array that carries same data, shouldn't be really seen as "immutable", right? Or, at least, I don't think we should penalize memory usage by forcing immutable data at all cost across all libraries, while having a "lowest memory consumption" common denominator seems helpful to also underline how much "immutable" costs, when used, or needed.

Still with that one change to add I think it could be useful to a large number of libraries

If most libraries can benefit from this, then I agree it doesn't have to cover 100% of all libraries, so it's OK to use it to simplify common things.

As for changing data with append, since data is the source of truth, I believe it should change only when items with-in changes too, which is a similar way you would add "infinite scroll", keep asking items to the server, but not invalidating all previously known item in doing so (as it's kinda pointless, and memory greedy).

This also would break tests that trust data either being new (clear, run, runLots), or keep growing (add), and since changing this behavior is trivial, doing it in the update callback, I'd rather keep it simple and efficient, memory speaking, and relational speaking, for the time being.

In few words, as long as data keeps existent item, its entity shouldn't change, while it should be always different, when its items are new/re-created.

After all, that's exactly what's reflected on the DOM, the tbody list of tr, is the data equivalent: it changes entirely only when data changes too, it adds to the same list of nodes, when new nodes are appended.

WebReflection commented 4 years ago

P.S. would State(callback, immutable = false) be a good way to solve the push vs concat debate?

ryansolid commented 4 years ago

Sorry I'm completely mistaken your approach is completely consistent with mutation. It is true some libraries are design for immutable data and would need different operations but like you showed could probably handle it or a different version. The place where it gets a little bit dicey is that some of the libraries that are based off immutability have the ability to still work on the mutation case in some cases and as you highlighted it can reduce memory overhead. So there are optimizations there which might be inconsistent. But in general what you have posted is consistent that was my mistake. And making an immutable version would require changes to more than just add.

But at the heart those are the three main approaches. One that uses immutable data structures and referential equality to diff and requires keys to handle list reconciliation. One that uses mutation and checks every Leaf value when it diffs however if can use references instead of keys for list reconciliation. And one that uses reactive data that does not diff necessarily and only updates the corresponding DOM on change.

brodybits commented 4 years ago

How about another idea: for each major approach we make a test suite similar to https://github.com/webcomponents/custom-elements-everywhere where we can compare the frameworks in ability and performance to provide React-like functionality, Surplus-like functionality, etc. etc.

Does this idea make any sense?

WebReflection commented 4 years ago

making an immutable version would require changes to more than just add.

I've just pushed a version that accepts an optional second argument that, if true (or truthy) will use immutability, consisting of:

As nothing else is mutated in this benchmark, I think the utils now would offer great primitives for most cases.

ryansolid commented 4 years ago

@brodybits I'm not sure I follow the suggestion. Custom Elements Everywhere much like this benchmark has a common goal that all libraries try to implement in their own way to fit some set requirement. The goal isn't to be like any other particular library but to achieve ultimately the same DOM state. Differently libraries do it in different incompatible ways but they basically end up the same. It might be worth talking about tagging implementations or some sort of sub categorization but I don't know how feasible that is.

I think the challenge here is that there is always hybridization. Like Reactive libraries using Virtual DOM diff techniques on list reconciliation or Immutable libraries using mutation in some cases because they work out better in this particular test. Reactive libraries are particularly challenging to have any set data objects. But I think it is a given this type of approach would never fly for this category.

@WebReflection Nice that was quick work. I think that covers as a base most non-reactive libraries. Whether someone using reducers or Redux would opt into that is another thing but with the immutable option it works pretty much straight out of the box by mapping action to function call.

brodybits commented 4 years ago

@ryansolid I will try to explain what I meant:

It seems like there are multiple approaches such as Surplus-like DOM, React-style VDOM, etc. My idea was just to group libraries together, based on which set of requirements they fulfill, for the sake of easier comparison. I think it would be possible for some libraries to fit into multiple groups. Does this make any more sense?

brodybits commented 4 years ago

My idea was just to group libraries together, depending on which set of requirements they fulfill, for the sake of easier comparison. It should be possible for some but not all libraries to fit into multiple groups. Does this make any more sense?

P.S. In response to the comment below, I was only trying to help solve the underlying issue which is that comparison is not so straightforward due to the variety of approaches that are covered by this benchmark. My idea was just to help resolve this issue somehow. Over and out.

WebReflection commented 4 years ago

can I ask to please keep this thread in topic? The benchmark has 6 buttons that do the same thing in every implementation: add, clear, create, create-many, swap, or update.

I am not suggesting to touch this framework by an inch, I am suggesting to normalize the common underlying functionalities instead of repeating these all over, with extra effort for reviews, merge, etc.

I don't think grouping frameworks has anything to do with this, so please let's discuss what this issue was created for, thank you.

WebReflection commented 4 years ago

@brodybits emoji are fun and everything, but if you could explain what has your grouping proposal to do with the rest of this issue, maybe I could try to contribute with the conversation, in case I haven't got it ... but I think it's off-topic, as it's not addressing what this issue is trying to address, or does it?

brodybits commented 4 years ago

I will try to explain in response. I think we know that different frameworks have different APIs in many (many) cases. In some cases such as React and Preact the APIs are almost the same, in other cases such as React and Surplus the API could be quite different. I think Surplus has a somewhat different API based on S.js which is needed since it does not support VDOM.

My idea was just to consider if we could use grouping to help keep same or similar test code for libraries that are very similar, such as React and Preact.

If it doesn't make sense, just ignore my comments.

WebReflection commented 4 years ago

I think that covers as a base most non-reactive libraries.

@ryansolid thanks for the quick review/feedback. Well, the module is published at this point, all I'd like to know is if @krausest is OK with me using it, and in that case, if he'll wait for my changes before considering merging my 5 PRs.

It could also be a good opportunity to show how the helper works in various scenarios, as it's used in raw diffing libraries, hooks based libraries, ad Custom Elements libraries too.

WebReflection commented 4 years ago

If it doesn't make sense, just ignore my comments.

my only doubt is that every framework whatsoever will use those 6 buttons and trigger 6 actions, all exposed through this utility.

It doesn't really matter how they create the buttons, the click will trigger same actions all over. The way data is created is also always the same all over, so ... do we need any extra grouping to always do the same thing, no matter what?

The current functionality serves as a data "setter" too, as example, the callback is invoked every time a button changed and the data also changed.

However, maybe I can expose buildData(length) too, so that frameworks that want to handle actions and data themselves, can do that.

krausest commented 4 years ago

@WebReflection I'm absolutely fine with the common utility module (but I don't think I'll enforce its usage for all js frameworks).

WebReflection commented 4 years ago

@krausest awesome. Then please wait for my changes to land in all my libraries, thank you

WebReflection commented 4 years ago

FYI the js-framework-benchmark-utils module exposes both State (previously Scope default) utility, and buildData, so that at least framework testers can use either to simplify their setup.

import {State, buildData} from 'js-framework-benchmark-utils';

I am still planning to use this in every of my libraries in line as PR, so bear with me, I'll do it ASAP

WebReflection commented 4 years ago

@krausest just FYI all my 5 PRs are ready to land. I've used js-framework-benchmark-utils in all of them, and also described a particular case with hooks and state, handled via neverland but working anyway.

It's worth saying that my last days of investigation with this benchmark brought under my attention a variety of issues, most noticeable the cost of WeakMap, which I have previously used without thinking too much, as I couldn't expect such big slowdown when these contains many weak references as key.

This means that I have to work to optimize as much as possible augmentor, as if there's any chance to avoid using a WeakMap in useState, I'll go for it, as the selectRow tests shows these are the bottleneck for both neverland and heresy.

That being said, I am overly satisfied about current state of my libraries, especially 'cause I've found out that hyperHTML test case was using performance.now() all over the place, it wasn't minized properly, and there was a lot of wrong things with it, so that now it seems like it's faster than lit or lighter html in the keyed result 😅

TL;DR please do merge those PRs whenever you have time, but I hope in a month (busy time ahead) I can just bump neverland and heresy versions to see way better results not ruined by the selectRow score.

Thanks again for patience, hints, and for welcoming the 3rd parts utils library proposal 👋

luwes commented 4 years ago

I got a lot of good stuff from the Preact hooks implementation, might be of interest to you. No WeakMap's I believe. Ended up using a adapted version in Swiss after I used Augmentor, https://github.com/luwes/swiss/blob/master/packages/swiss/hooks/src/core-hooks.js

WebReflection commented 4 years ago

@luwes thanks for the hint, but it's not a library by itself and, after a quick scan, it doesn't look at feature-pair with React, while augmentor does both: it's a library by its own, with 100% code coverage, and it respects all React hooks concepts (i.e. lazy setState initialization, absent in there, but also no try/catch around hooks, it looks a bit premature as production ready implementation).

Oddly enough, I'm pretty sure Swiss used augmentor at its beginning, sad to see it created its own hooks instead of helping with the project.

luwes commented 4 years ago

Yes, it's optimized for use with custom elements and small size (adapted from Preact hooks). I decided to remove all 3rd party dependencies after a while. Got carried away with bundle size optimization (it's 1kB), finer control and code readability.

Anyway just wanted to point out WeakMap-less hacks/implementations.

WebReflection commented 4 years ago

I already fixed that in augmentor at least where is not necessary (useState, useReducer) ... but I'll benchmark tomorrow. I expect neverland to bump from 70 to ~35, in select row, which would be a win, if that's the case

WebReflection commented 4 years ago

I expect neverland to bump from 70 to ~35,

welp ... that didn't happen, there's still double arguments loop slowdown, but at least now it's around 50+ which seems better, so augmentor (and dom-augmentor) are not the bottleneck anymore, meaning I won't push any more changes in that regard.

neverland might need a massive rewrite, copy pasting lighterhtml up to the loop and changing that bit but ... hey, nothing I'll rush doing any time soon, as performance are super good already anyway 😊

@krausest just merge whenever you'll have time, I am not planning to change much in the near future.

Thank you all for hints and thoughts, if nothing, this project shows that competing can be polite and super productive 🎉

Best Regards