luwes / js-diff-benchmark

Simple benchmark for testing your DOM diffing algorithm.
66 stars 5 forks source link

Relative spect impl #29

Closed dy closed 4 years ago

dy commented 4 years ago

Ok, [hopefully] the latest spect implementation, 209b. This version allows passing live parent.childNodes collection as a (no need for slicing), so a is optional. Also that doesn't need before, because that's forward algorithm. Also the digits seem to be better now. image

luwes commented 4 years ago

feel free to merge, added you as collaborator.

dy commented 4 years ago

Interesting fact: removing swap check makes it 185b with a bit worse swap-tests performance (but still workable!): image

dy commented 4 years ago

@luwes may I ask you to take a new screenshot?

luwes commented 4 years ago

👍 will do

luwes commented 4 years ago

strange I can't get spect on 4th :( it's always 5th or last on my computer.

Screen Shot 2020-04-18 at 2 45 19 PM Screen Shot 2020-04-18 at 2 49 00 PM
luwes commented 4 years ago

I might look into adding this to a CI later, take those results as a baseline

dy commented 4 years ago

Measured that from my laptop & derived as ratio from competitors. Performance fluctuates, also benchmark is not optimized, so it doesn't matter much tbh, moved to 5 place & updated image.

WebReflection commented 4 years ago

It's clear we need a real browser to stop complaining about dummy.js, however, focusing on bytes only has its own cost.

I don't think having 100 bytes implementation is a win, if it doesn't perform, as the issue out there is not a bounce of bytes.

Also, the before is optional, but essential, so if a library makes a container "overflow" is not really great, IMHO, but so far that's not a metric.

dy commented 4 years ago

if it doesn't perform, as the issue out there is not a bounce of bytes

Timing is euphemeral - you switch lines order, loop/data type, node version, browser, engine, platform, hardware - and it fluctuates, so what that measures? Environment itself? Complexity is what matters first of all, and that tends to manifest in canonical forms of algorithms. There's nothing super complex about diffing two collections, it is similar to unraveling a thread in real world, and that can't be done faster than O(c *n) (looking forward for proof of the opposite). My interest in benchmark is not figuring out the amount of c, or stuffing code with heuristics, but finding that canonical optimal form of algorithm, that can be seasoned by flavor (caching/keys/size/shortcuts etc) in applications.

library makes a container "overflow"

Could you elaborate? Not sure I see point in stuffing input with variables that can be derived from data itself. before is not essential, since the b array unambiguously dictates the output sequence, and a provides the exact items and input sequence. Moreover before presumes that the algo is backward-oriented and relies on insertBefore - but why not after, for example?

WebReflection commented 4 years ago

The assumption the whole container content can be destroyed is not valid, as you can diff a container that already has nodes in it (hydration) and confine your diffing only in one specific point, which is the one where JSX, template literals, or any other technique, decides to diff 'cause the "hole" accepts an array, which can be before any other node.

<div>
  <!--diff here-->
  <p>the rest</p>
</div>
WebReflection commented 4 years ago

I guess my point is that you keep complaining about nextSibling which is used by every other library ... once we use also a real browser, we'll see the difference but so far your golfing is causing worse performance each iteration. If you have the best algo in 200 bytes hat tip, but maybe until you can prove it there's no need to keep blaming the benchmark, as every other library deals with exact same constraints, would you agree?

dy commented 4 years ago

"hole" accepts an array, which can be before any other node.

Ok, I see. Can be done as diff(parent, [before], [a,b,c,before]) though.

WebReflection commented 4 years ago

That requires a slice on the target list anyway, while the benchmark is keeping before optional as parameter to optionally consider. Feel free to ignore it, but if it's there, and outside the diffing logic that wouldn't care about it, but consider it as boundary, the library should be capable of not trespassing that boundary. All libraries do that do far, so I'm not sure what are you after.

dy commented 4 years ago

what are you after

Just to illustrate that before forces pin element in container, if that happens to diff-insert after some content:

<div>abc</div>
        ↑
     here

Not sure what's right - maintaining placeholders or slicing nodes to diff.

WebReflection commented 4 years ago

the before element is just a fallback for nextSibling when it's null (off-tree node to be inserted), but again, it's not used in the default benchmark, but if having it in the array is the issue, put it in the array before operating, or place it as the last element of the map.

I don't see it as an issue and all implementation works just fine with it.

Anyway, I'll try to create a browser based table so at least we can compare on real DOM engine.