mirror of
https://github.com/neovim/neovim.git
synced 2026-08-25 00:21:55 +00:00
vim-patch:9.2.0937: sort() with a numeric option converts each item on every comparison (#41286)
Problem: sort() with "n", "N" or "f" converts an item to its number on
every comparison. For "n" that is a tv2string() plus strtod()
per comparison, so sorting a list of numbers turns each number
into a string and back O(n log n) times, dwarfing the sort.
Solution: Compute the numeric key of each item once, before the sort,
and compare the stored key (Samuel Schlesinger). Only the
builtin numeric compare modes are affected; uniq(), which
passes a bare list item to the compare function, and the
string and user-function paths are unchanged.
Sorting a list of 100000 numbers (min of 3, macOS arm64):
- sort(l, 'n'): 0.205s -> 0.017s
- sort(l, 'N'): 0.017s -> 0.010s
- sort(l, 'f'): 0.014s -> 0.010s
The result is identical, including that a string is still treated as 0
in "n" mode and that "N" keeps full 64-bit precision.
Add Test_sort_numeric_precomputed(): a large shuffled list sorted with
"n", mixed integers and floats, int64 values beyond the exact range of
a double for "N", and uniq() over the non-precomputed path.
closes: vim/vim#21003
c8c59db9df
Co-authored-by: Samuel Schlesinger <sgschlesinger@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -52,12 +52,21 @@ typedef struct {
|
||||
partial_T *item_compare_partial;
|
||||
dict_T *item_compare_selfdict;
|
||||
bool item_compare_func_err;
|
||||
bool item_compare_keys_ready; ///< ptrs[].key is precomputed
|
||||
} sortinfo_T;
|
||||
|
||||
/// Structure representing one list item, used for sort array.
|
||||
typedef struct {
|
||||
listitem_T *item; ///< Sorted list item.
|
||||
int idx; ///< Sorted list item index.
|
||||
/// Sort key precomputed once per item for the numeric compare modes, so
|
||||
/// item_compare() does not convert the value on every comparison. Only
|
||||
/// valid when sortinfo->item_compare_keys_ready is set (the sort() path);
|
||||
/// uniq() passes a bare listitem_T pointer and must not read this.
|
||||
union {
|
||||
varnumber_T inum; ///< for item_compare_numbers ("N")
|
||||
double fnum; ///< for item_compare_numeric ("n") and _float ("f")
|
||||
} key;
|
||||
} ListSortItem;
|
||||
|
||||
typedef int (*ListSorter)(const void *, const void *);
|
||||
@@ -1193,21 +1202,36 @@ static int item_compare(const void *s1, const void *s2, bool keep_zero)
|
||||
int res;
|
||||
|
||||
if (sortinfo->item_compare_numbers) {
|
||||
const varnumber_T v1 = tv_get_number(tv1);
|
||||
const varnumber_T v2 = tv_get_number(tv2);
|
||||
const varnumber_T v1 = sortinfo->item_compare_keys_ready
|
||||
? si1->key.inum : tv_get_number(tv1);
|
||||
const varnumber_T v2 = sortinfo->item_compare_keys_ready
|
||||
? si2->key.inum : tv_get_number(tv2);
|
||||
|
||||
res = v1 == v2 ? 0 : v1 > v2 ? 1 : -1;
|
||||
goto item_compare_end;
|
||||
}
|
||||
|
||||
if (sortinfo->item_compare_float) {
|
||||
const float_T v1 = tv_get_float(tv1);
|
||||
const float_T v2 = tv_get_float(tv2);
|
||||
const float_T v1 = sortinfo->item_compare_keys_ready
|
||||
? si1->key.fnum : tv_get_float(tv1);
|
||||
const float_T v2 = sortinfo->item_compare_keys_ready
|
||||
? si2->key.fnum : tv_get_float(tv2);
|
||||
|
||||
res = v1 == v2 ? 0 : v1 > v2 ? 1 : -1;
|
||||
goto item_compare_end;
|
||||
}
|
||||
|
||||
if (sortinfo->item_compare_numeric && sortinfo->item_compare_keys_ready) {
|
||||
double n1 = si1->key.fnum;
|
||||
double n2 = si2->key.fnum;
|
||||
|
||||
res = n1 == n2 ? 0 : n1 > n2 ? 1 : -1;
|
||||
if (res == 0 && !keep_zero) {
|
||||
res = si1->idx > si2->idx ? 1 : -1;
|
||||
}
|
||||
return res;
|
||||
}
|
||||
|
||||
char *tofree1 = NULL;
|
||||
char *tofree2 = NULL;
|
||||
char *p1;
|
||||
@@ -1345,6 +1369,38 @@ static int item_compare2_not_keeping_zero(const void *s1, const void *s2)
|
||||
return item_compare2(s1, s2, false);
|
||||
}
|
||||
|
||||
/// Precompute the numeric sort key of each item, so that item_compare() can
|
||||
/// compare the stored value instead of converting the item on every one of the
|
||||
/// O(n log n) comparisons. Only for the builtin numeric compare modes; each
|
||||
/// key is computed exactly as item_compare() would have, once per item.
|
||||
static void sort_compute_keys(ListSortItem *ptrs, int len, sortinfo_T *info)
|
||||
{
|
||||
if (info->item_compare_numbers) {
|
||||
for (int i = 0; i < len; i++) {
|
||||
ptrs[i].key.inum = tv_get_number(&ptrs[i].item->li_tv);
|
||||
}
|
||||
} else if (info->item_compare_float) {
|
||||
for (int i = 0; i < len; i++) {
|
||||
ptrs[i].key.fnum = tv_get_float(&ptrs[i].item->li_tv);
|
||||
}
|
||||
} else { // info->item_compare_numeric
|
||||
for (int i = 0; i < len; i++) {
|
||||
typval_T *tv = &ptrs[i].item->li_tv;
|
||||
|
||||
// A string is compared as a single quote in numeric mode, which
|
||||
// strtod() reads as 0; only numbers contribute a value.
|
||||
if (tv->v_type == VAR_STRING) {
|
||||
ptrs[i].key.fnum = 0.0;
|
||||
} else {
|
||||
char *p = encode_tv2string(tv, NULL);
|
||||
ptrs[i].key.fnum = p == NULL ? 0.0 : strtod(p, NULL);
|
||||
xfree(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
info->item_compare_keys_ready = true;
|
||||
}
|
||||
|
||||
/// sort() List "l"
|
||||
static void do_sort(list_T *l, sortinfo_T *info)
|
||||
{
|
||||
@@ -1362,6 +1418,14 @@ static void do_sort(list_T *l, sortinfo_T *info)
|
||||
});
|
||||
|
||||
info->item_compare_func_err = false;
|
||||
info->item_compare_keys_ready = false;
|
||||
// For the builtin numeric compares, precompute each item's key once
|
||||
// instead of converting it on every comparison.
|
||||
if (info->item_compare_func == NULL && info->item_compare_partial == NULL
|
||||
&& (info->item_compare_numbers || info->item_compare_float
|
||||
|| info->item_compare_numeric)) {
|
||||
sort_compute_keys(ptrs, len, info);
|
||||
}
|
||||
ListSorter item_compare_func = ((info->item_compare_func == NULL
|
||||
&& info->item_compare_partial == NULL)
|
||||
? item_compare_not_keeping_zero
|
||||
@@ -1429,6 +1493,7 @@ static int parse_sort_uniq_args(typval_T *argvars, sortinfo_T *info)
|
||||
info->item_compare_func = NULL;
|
||||
info->item_compare_partial = NULL;
|
||||
info->item_compare_selfdict = NULL;
|
||||
info->item_compare_keys_ready = false;
|
||||
|
||||
if (argvars[1].v_type == VAR_UNKNOWN) {
|
||||
return OK;
|
||||
|
||||
@@ -53,6 +53,9 @@ func Test_sort_numeric()
|
||||
call assert_equal([3, 13, 28], sort([13, 28, 3], 'n'))
|
||||
" strings are not sorted
|
||||
call assert_equal(['13', '28', '3'], sort(['13', '28', '3'], 'n'))
|
||||
" a string sorts as 0 in "n" mode, even a numeric-looking one
|
||||
call assert_equal(['a', 0, 1], sort([1, 'a', 0], 'n'))
|
||||
call assert_equal(['10', 2], sort([2, '10'], 'n'))
|
||||
endfunc
|
||||
|
||||
func Test_sort_numbers()
|
||||
@@ -66,6 +69,34 @@ func Test_sort_float()
|
||||
call assert_equal([0.28, 3, 13.5], sort([13.5, 0.28, 3], 'f'))
|
||||
endfunc
|
||||
|
||||
" The numeric compare modes precompute a sort key per item; exercise that on
|
||||
" larger lists (many comparisons), with mixed int/float, negatives, and
|
||||
" int64 values that do not fit in a double for 'N'.
|
||||
func Test_sort_numeric_precomputed()
|
||||
" 'n' on a large shuffled list, compared against the known order.
|
||||
let expected = range(500)
|
||||
let shuffled = copy(expected)
|
||||
" deterministic shuffle
|
||||
let s = 7
|
||||
for i in range(len(shuffled) - 1, 1, -1)
|
||||
let s = (s * 1103515245 + 12345) % 2147483648
|
||||
let j = s % (i + 1)
|
||||
let [shuffled[i], shuffled[j]] = [shuffled[j], shuffled[i]]
|
||||
endfor
|
||||
call assert_equal(expected, sort(shuffled, 'n'))
|
||||
|
||||
" 'n' with mixed integers and floats and negatives.
|
||||
call assert_equal([-3, -1.5, 0, 0.5, 2, 7.25], sort([7.25, -1.5, 2, 0, -3, 0.5], 'n'))
|
||||
|
||||
" 'N' with int64 values beyond the exact range of a double: keys must not
|
||||
" be rounded through a double.
|
||||
call assert_equal([9007199254740992, 9007199254740993, 9007199254740994],
|
||||
\ sort([9007199254740994, 9007199254740992, 9007199254740993], 'N'))
|
||||
|
||||
" uniq() uses the non-precomputed path; it must still work.
|
||||
call assert_equal([1, 2, 3], uniq(sort([3, 1, 2, 2, 3, 1], 'n')))
|
||||
endfunc
|
||||
|
||||
func Test_sort_nested()
|
||||
" test ability to call sort() from a compare function
|
||||
call assert_equal([1, 3, 5], sort([3, 1, 5], 'Compare1'))
|
||||
|
||||
Reference in New Issue
Block a user