WebReflection commented 6 years ago

The current hyperHTML does not show up in the server list and it's not configured in an ideal way.

This PR would like to bring latest hyperHTML which is also oriented to performance rather than correct amount of swaps.

Please consider this merge to make it part of this suite, thanks.

WebReflection commented 6 years ago

Update I've squashed/merged the keyed version too

WebReflection commented 6 years ago

Hey @krausest , do you have any idea what's going on with the store ?

krausest commented 6 years ago

Just merged. The store was updated since the swap benchmark now swaps different rows (#310). You non-keyed version behaves according to my webdriver-ts/src/nonKeyed.ts test as keyed. Do you want to keep it and adjust? (Who's proud of a non-keyed impl anyways ;-)

hyperhtml-v2.1.2-non-keyed is keyed for 'run benchmark' and keyed for 'remove row benchmark' keyed for 'swap rows benchmark' . It'll appear as keyed in the results
ERROR: Framework hyperhtml-v2.1.2-non-keyed is not correctly categorized in commons.ts

Building webdriver-ts still build fine on my machine.

WebReflection commented 6 years ago

hey @krausest there is an issue with #310 in my opinion .. and it messes up my tests ...

if(this.data.length > 10) {
  // before
  this.data[4] = this.data[9];
  // NOW ?!?!?!?!??!
  this.data[1] = this.data[998];
}

so, if the length is more than 10 it's OK to assume this.data[998] exists at all?

That looks like a hell of an issue with reflex-dom which is anyway very slow/non-competitive ... I'd consider reverting that thing or update the if statement or, in case you believe that's OK, explain what every framework is supposed to do there 'cause all I see are 989+ potential holes in the data array.

You non-keyed version behaves according to my webdriver-ts/src/nonKeyed.ts test as keyed.

The thing with hyperHTML is that it makes no sense non-keyed but the current non-keyed version does not fully use the key like the keyed version does via the Component (I know it's hard to explain this sentence ... bear with me ...)

Do you want to keep it and adjust?

What's the non-keyed version deal? Same nodes updated with different data and different event listeners?

If that's the case, I rather drop hyperHTML from non-keyed version becauuse it won't ever reasonably work there but if that's what's going to happen, for benchmark sake, I might later on also improve my current keyed demo to avoid components and use the least amount of resources to work as expected.

Please let me know, thanks.

krausest commented 6 years ago

You're right. The condition makes no longer sense. I opened #312 for that. I definitely want to keep that change since it shows a weakness for some frameworks that the old didn't. Reflex-dom is definitely special... According to nonKeyed.ts an implementation is keyed if all of the following holds:

Swap rows creates and deletes TR elements
The remove operation removes exactly the TR that was clicked
Update 1,000 rows creates and removes TR elements Your non-keyed version does each from the above :sunglasses:

So I suggest removing the non-keyed version. Improvements to your keyed version are welcome unless @leeoniya finds you cheating :stuck_out_tongue_winking_eye:

WebReflection commented 6 years ago

Your non-keyed version does each from the above

if the state object passed along is refreshed, swapped, updated, hyperHTML would never assign a different DOM node to it. If the same state is removed/replaced, then yes, hyperHTML would create/replace new content for the new state.

Happy to remove the non-keyed version.

However I am not happy to include unrealistic holes in the test. A broken swap of states that points to undefined states, instead of removing them, is not a goal of any of the libraries in this benchmark.

Why do you think that test is necessary?

WebReflection commented 6 years ago

After reading more ... I'm a bit confused ...

I definitely want to keep that change since it shows a weakness for some frameworks that the old didn't.

but apparently in 320 you are saying that the if should be improved to be sure no 997 potential holes are created, right?

Is the next update to have that if (dta.length > 998) {... or I misunderstood everything?

krausest commented 6 years ago

Yes, that's the condition I'm adding.

WebReflection commented 6 years ago

cool, so I'll try to avoid cheating and PR the changes to make non-keyed a ghost and current keyed a bench-release version, thanks

WebReflection commented 6 years ago

all of that, after you'll push the current store change though :rofl:

leeoniya commented 6 years ago

I might later on also improve my current keyed demo to avoid components and use the least amount of resources to work as expected.

in domvm's bench impl, i also avoid creating a new component for each row to save on resources (all components are technically stateful in domvm). while this is within the guidelines of the bench, it does feel a bit like cheating in one sense but at the same time many frameworks use components because they're the only way to achieve some optimizations or behavior (sCU or keying). other impls use components because it's more idiomatic. other dom patching libs have no concept of components at all. it's a gray area that's made even more gray by having to render an unrealistic 80,000 dom nodes (which any real world impl would not do, or try to optimize to the max anyways). this bench aims for just the right balance of prescriptivism and masochism :D

WebReflection commented 6 years ago

it does feel a bit like cheating

well, in hyperHTML the Component is the least used mechanism, not the primary, indeed I pushed a regardless keyed version of the library in between non keyed tests ... all I want to do is to use same "non-keyed-but-actually-keyed" version, which is the most natural, avoiding creation of new handler per each key, which is within the guidelines, 'cause that's what my current keyed version does.

I hope that's fine, 'cause that's how hyperHTML works anyway

WebReflection commented 6 years ago

this bench aims for just the right balance of prescriptivism and masochism :D

absolutely agree on that, this bench makes almost no sense to me because any app, Web based or native, that shows 10.000 rows at once, is a bad app, no matter how fast is the machine that shows it :smile:

leeoniya commented 6 years ago

here's generally what i watch for

is the impl data-driven without having to modify the dom imperatively in ways specific to this bench
is the dom reconcile algorithm generic without extra tweaks necessary to score well here
does the lib do the right thing for all ops (this is mostly solved by the nonKeyed test)
are optimizations in the bench impl generally applicable to other dom shapes without much tweaking.

there are still some fast libs here that very much walk the line. surplus had a separate reconcile algo in app-space for the non-keyed bench. i think it's fixed now. redom does some direct dom mutation but i discovered that it's only declarative for creation and highly imperative for updates. it's hard to draw the line somewhere when so many different framework philosophies must be accomodated.

WebReflection commented 6 years ago

here my PR for the real hyperHTML version of this test: https://github.com/krausest/js-framework-benchmark/pull/313

alexfmpe commented 6 years ago

That looks like a hell of an issue with reflex-dom which is anyway very slow/non-competitive

Thought I'd clarify a bit.

310 wasn't a hack to accommodate reflex-dom issues.

It simply didn't update the if length > condition, which was fixed in #312.

Reflex-dom is indeed a special case (at least for the time being). The benchmark is actually merged on the framework's repository, and npm install clones that repository inside the reflex-dom directory in this one. This is what caused @leeoniya to leave it out in #310.

krausest / js-framework-benchmark

Update hyperHTML v2.1 #309

310 wasn't a hack to accommodate reflex-dom issues.