theKashey / proxyequal

There is a bit more smart way to compare things, than a shallow equal.
MIT License
72 stars 7 forks source link

Better nestedSort #16

Closed dai-shi closed 5 years ago

dai-shi commented 5 years ago

I guess nestedSort was not implemented as expected.

> [".e",".e.a",".e.a.b",".a",'.b'].sort(old_nestedSort)
[ '.e', '.a', '.b', '.e.a', '.e.a.b' ]
> [".e",".e.a",".e.a.b",".a",'.b'].sort(new_nestedSort)
[ '.a', '.b', '.e', '.e.a', '.e.a.b' ]

weakMemoization should be disabled for benchmarking nestedSort.

old: proxyShallowEqual x 390,346 ops/sec ±1.98% (87 runs sampled) new: proxyShallowEqual x 424,974 ops/sec ±0.72% (89 runs sampled)

theKashey commented 5 years ago

I was trying to implement non alphabetic Nat sort to make it a bit faster, as long as artsy is already “preordered”. I really did test it for some edge cases.

dai-shi commented 5 years ago

Don’t you want .e to be right before .e.a?

theKashey commented 5 years ago

Yep. Not strictly alphabetically, but strictly nested.

theKashey commented 5 years ago

Fuck! My algorithm was correct.

if (al < bl) {
    return b.indexOf(a) === 0 ? -1 : 1;
  }
  if (al > bl) {
    return a.indexOf(b) === 0 ? 1 : -1;
  }

This is how it was. But dirung the final code clearance I've inverted one line. So this is how it IS:

  if (al < bl) {
    return b.indexOf(a) === 0 ? -1 : 1;
  }
  if (al > bl) {
    return a.indexOf(b) === 0 ? -1 : 1;
  }
  return 0;
theKashey commented 5 years ago

🙌 I'll run performance benchmarks between two algos and pick the fastest one.

PS: And thank you for saving a day.

theKashey commented 5 years ago

In short - my one is twice faster, but qsort is not checking every possible pair. As a result - if two similar records were too far away from each other - they would not be ever compared and sorted. In your case alphabetical sorting would take more time, but will place values next to each other, as they should.

theKashey commented 5 years ago

It made 9 comparisons for an array of size 10 🤷‍♂️ in my case and 20 in yours (thus sorted it properly)

theKashey commented 5 years ago

If I "fix" my algo - it would make 32(!) comparison for an array of size 10. It would apply fewer changes to the final array, but would work much slower.

dai-shi commented 5 years ago

qsort is not checking every possible pair.

Yeah, that's what I thought.

theKashey commented 5 years ago

2.0.5 released. Thank you.