[webkit-reviews] review requested: [Bug 100754] Faster sorting of numeric arrays : [Attachment 172150] Patch and tests

[bugzilla-daemon at webkit.org]

Cosmin Truta <ctruta at gmail.com> has asked  for review:
Bug 100754: Faster sorting of numeric arrays
https://bugs.webkit.org/show_bug.cgi?id=100754

Attachment 172150: Patch and tests
https://bugs.webkit.org/attachment.cgi?id=172150&action=review

------- Additional Comments from Cosmin Truta <ctruta at gmail.com>
New submission, addressing Darin's comments.

(In reply to comment #10)
> (In reply to comment #9)
> > I’m not sure that this is the right way to convert from const
WriteBarrier<Unknown>& to JSValue*, even though I understand this is analogous
to what the function we passed to qsort did. Is there other code that does this
with these same two static_cast? Could you look at existing code to see what
the usual idiom is?
> I initially tried the proper conversion, i.e. "a.get().asNumber()", but the
end result was slower, not sure why though. I will give it another look.

I had noticed a small performance degradation last time I used this call, so I
kept the cast. Maybe something was wrong in my testing, because this time
around I rerun the tests, and I saw no difference in performance between the
proper conversion and the ugly cast.

(In reply to comment #9)
> It would be good to know what platform you did performance testing on, and
what the results are on the other most-dissimilar platforms.

Forgot to answer that.
My reported perf improvements were obtained on an Intel i7 running Linux Mint,
building JSC with gcc.
Moreover, I compared the raw performance between std::sort and qsort (in plain
C++, not JSC) using clang, and I had also run in the past such comparison tests
on other platforms, including Windows (both Visual C++ and MinGW). The
improvements given by std::sort were visible and consistent.
My experience is not singular. See, for example:
http://www.lemoda.net/c/inline-qsort-example/