New issue
Advanced search Search tips

Issue 910818 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 16
Cc:
Components:
EstimatedDays: ----
NextAction: 2019-01-16
OS: Linux , Windows , Mac
Pri: 2
Type: Feature



Sign in to add a comment

Array.sort makes duplicate comparisons

Reported by david.w...@gmail.com, Dec 1

Issue description

UserAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce the problem:
1. Create a custom sort function to count comparisons
2. Call [0, 2, 1].sort(countFunc)
3. Notice that 4 comparisons were used, when there are only 3 pairwise combinations of elements.

What is the expected behavior?
Array.sort should not make the same comparison twice. Permutations of length 3 should be sorted in 2 or 3 steps.

What went wrong?
Timsort makes the same comparison multiple times. The attached `index.html` demonstrates the problem for arrays of length 3. This issue can lead to thousands of extra comparisons on large arrays.

I filed a parallel bug with Python[1], which is the model for Chromium's Torque implementation[2].

The `sort-fix.diff` attachment is my proposed fix for Python. It may be worth following the other bug to see if people reject or improve it.

[1] https://bugs.python.org/issue35369
[2] https://v8.dev/blog/array-sort

Did this work before? No 

Chrome version: 70.0.3538.77 (Official Build) Built on Ubuntu , running on Ubuntu 18.04 (64-bit)  Channel: stable
OS Version: Ubuntu 18.04
Flash Version:
 
index.html
883 bytes View Download
sort-fix.diff
2.5 KB Download
TE@, this may be considered a feature request for someone from  bug 896385  or  bug 900651  e.g. jgruber@
Labels: Needs-Triage-M70
Cc: viswa.karala@chromium.org
Labels: -Type-Bug Triage-ET Target-73 M-73 FoundIn-71 FoundIn-70 FoundIn-73 FoundIn-72 OS-Mac OS-Windows Type-Feature
Status: Untriaged (was: Unconfirmed)
Thanks for filing the issue!

As per comment# 1, considering this issue as Feature request and marking it as Untriage.

Thanks!
Components: -Blink Blink>JavaScript
Owner: jgruber@chromium.org
Status: Assigned (was: Untriaged)
Cc: jgruber@chromium.org
NextAction: 2019-01-16
Owner: szuend@google.com
Handing this off to szuend@. Note that there's no bug here, this is just how TimSort works. Tim Peters' reply in the linked python bug sounds like no changes to TimSort will be forthcoming.

Comment 7 by szuend@google.com, Jan 16 (6 days ago)

Status: WontFix (was: Assigned)
Thanks for filing!

After looking into the diff and the analysis of Tim Peters, I have to agree with him. IMO, a minor improvement that probably won't be measurable in most cases, does not warrant the increased complexity, plus check for correctness.

Comment 8 by monor...@bugs.chromium.org, Jan 16 (6 days ago)

The NextAction date has arrived: 2019-01-16

Sign in to add a comment