1887697 - Avoid nsTArray<>::Sort to use ElementAt with bounds check while sorting

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Jens Stutte [:jstutte]]

Coming from bug 1848765 made me think, we should do the same as we do for StableSort, that is pass the plain C/C++ array via Elements() to std::sort to avoid bounds checking.

Comment 1

[Jens Stutte [:jstutte]]

Attachment: Bug 1887697 - Avoid nsTArray<>::Sort to use ElementAt with bounds check while sorting. r?#xpcom-reviewers

We are aware of the fact that std::sort does no bounds checks at all and
can be derailed by bad comparators, but we assume we will catch bad
comparators with tests in DEBUG builds and can go for full speed here.

Comment 2

[Andrew McCreight [:mccr8]]

Is "whith" in the summary supposed to be "without"?

Comment 3

[Jens Stutte [:jstutte]]

(In reply to Andrew McCreight [:mccr8] from comment #2)

Is "whith" in the summary supposed to be "without"?

It's supposed to be "with", given the "Avoid".

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4efdaac44d1
Avoid nsTArray<>::Sort to use ElementAt with bounds check while sorting. r=xpcom-reviewers,nika

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/f4efdaac44d1

Comment 6

[Alex Finder]

(In reply to Pulsebot from comment #4)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4efdaac44d1
Avoid nsTArray<>::Sort to use ElementAt with bounds check while sorting.
r=xpcom-reviewers,nika

== Change summary for alert #42015 (as of Tue, 26 Mar 2024 08:21:37 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
0.16%	installer size	osx-cross		93,960,921.83 -> 93,812,125.75

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=42015

Comment 7

[Jens Stutte [:jstutte]]

(In reply to Alex Finder from comment #6)

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
0.16%	installer size	osx-cross		93,960,921.83 -> 93,812,125.75

It seems we had previously more code from inlined ElementAt with bounds check than we need now, at least on Mac. That confirms the patch does something. Nice.