[stdlib][fix] Fixed wrong sorting behavior

[kchibisov]

Fix for this bug.

We were using base index instead of index from current iteration in _siftDown.

[xwu]

Glad the issue could be tracked down so quickly.

We need tests. At minimum, the original example in the bug report. But this raises concern for large gaps in testing and ideally a more systematic approach to closing them would be important.

[xwu]

If there is no reason ever to use the original value idx, then the first line of the function body should be var idx = idx. Even if the proposed change here fixes this particular bug, leaving both i and idx around is a risk for the next edit made to this function.

[airspeedswift]

@swift-ci please test

[airspeedswift]

@swift-ci please smoke benchmark

[swift-ci]

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
SortIntPyramid	8744	19892	+127.5%	0.44x
SortAdjacentIntPyramids	10215	11354	+11.2%	0.90x

Code size: -O

TEST	OLD	NEW	DELTA	SPEEDUP
Improvement
StaticArray.o	33033	32169	-2.6%	1.03x

Performance: -Osize

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
SortIntPyramid	8857	20034	+126.2%	0.44x
SortAdjacentIntPyramids	10704	12035	+12.4%	0.89x

Performance: -Onone

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
SortIntPyramid	245301	401446	+63.7%	0.61x

Code size: Swift libraries

TEST	OLD	NEW	DELTA	SPEEDUP
Improvement
libswiftSwiftOnoneSupport.dylib	221184	217088	-1.9%	1.02x

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false alarms. Unexpected regressions which are marked with '(?)' are probably noise. If you see regressions which you cannot explain you can try to run the benchmarks again. If regressions still show up, please consult with the performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

---

[xwu]

Hmm, looks like the original improvement in performance that was observed in #18629 originates from the bug and not from making the change from recursion to iteration.

[airspeedswift]

Merging to fix bug on mater, but we may need to add further tests on top of what's here as a follow-on.

[airspeedswift]

On the bright side, this restores my hope that we can have a stable sort that doesn't significantly lose out to this one in terms of performance :)