perf(sort): precompute sort keys and fast-path numeric sorter

src/js/modules/Sort/Sort.js

@@ -166,19 +166,20 @@ export default class Sort extends Module{
 				sorters=[],
 				match = false;
 
-				if(column.modules.sort){
-					if(column.modules.sort.tristate){
-						if(column.modules.sort.dir == "none"){
-							dir = column.modules.sort.startingDir;
+				const sortModule = column.modules.sort;
+				if(sortModule){
+					if(sortModule.tristate){
+						if(sortModule.dir == "none"){
+							dir = sortModule.startingDir;
 						}else{
-							if(column.modules.sort.dir == column.modules.sort.startingDir){
-								dir = column.modules.sort.dir == "asc" ? "desc" : "asc";
+							if(sortModule.dir == sortModule.startingDir){
+								dir = sortModule.dir == "asc" ? "desc" : "asc";
 							}else{
 								dir = "none";
 							}
 						}
 					}else{
-						switch(column.modules.sort.dir){
+						switch(sortModule.dir){
 							case "asc":
 								dir = "desc";
 								break;
@@ -188,7 +189,7 @@ export default class Sort extends Module{
 								break;
 
 							default:
-								dir = column.modules.sort.startingDir;
+								dir = sortModule.startingDir;
 						}
 					}
 
@@ -355,32 +356,29 @@ export default class Sort extends Module{
 		if(this.table.options.sortMode !== "remote"){
 
 			//build list of valid sorters and trigger column specific callbacks before sort begins
-			sortList.forEach(function(item, i){
-				var sortObj;
-
-				if(item.column){
-					sortObj = item.column.modules.sort;
-
+			for(const item of sortList) {
+				const column = item.column;
+				if(column){
+					const sortObj = column.modules.sort;
 					if(sortObj){
-
 						//if no sorter has been defined, take a guess
 						if(!sortObj.sorter){
-							sortObj.sorter = self.findSorter(item.column);
+							sortObj.sorter = self.findSorter(column);
 						}
 
-						item.params = typeof sortObj.params === "function" ? sortObj.params(item.column.getComponent(), item.dir) : sortObj.params;
+						item.params = typeof sortObj.params === "function" ? sortObj.params(column.getComponent(), item.dir) : sortObj.params;
 
 						sortListActual.push(item);
 					}
 
 					if(!sortOnly) {
-						self.setColumnHeader(item.column, item.dir);
+						self.setColumnHeader(column, item.dir);
 					}
 				}
-			});
+			}
 
 			//sort data
-			if (sortListActual.length) {
+			if (sortListActual.length && data.length) {
 				self._sortItems(data, sortListActual);
 			}
 
@@ -439,42 +437,57 @@ export default class Sort extends Module{
 
 	//sort each item in sort list
 	_sortItems(data, sortList){
-		var sorterCount = sortList.length - 1;
-
-		data.sort((a, b) => {
-			var result;
-
-			for(var i = sorterCount; i>= 0; i--){
-				let sortItem = sortList[i];
-
-				result = this._sortRow(a, b, sortItem.column, sortItem.dir, sortItem.params);
-
+		const sortMeta = sortList.map((sortItem) => {
+			return {
+				column: sortItem.column,
+				dir: sortItem.dir,
+				params: sortItem.params,
+				sorter: sortItem.column.modules.sort.sorter,
+				columnComponent: sortItem.column.getComponent(),
+				asc: sortItem.dir === "asc",
+			};
+		});
+
+		const sorterCount = sortMeta.length - 1;
+		const length = data.length;
+
+		//extract each row's sort keys and component once, then sort the decorated array
+		const decorated = new Array(length);
+		for(let k = 0; k < length; k++){
+			const row = data[k];
+			const rowData = row.getData();
+			const values = new Array(sortMeta.length);
+			for(let j = 0; j <= sorterCount; j++){
+				const value = sortMeta[j].column.getFieldValue(rowData);
+				values[j] = typeof value !== "undefined" ? value : "";
+			}
+			decorated[k] = {values, component: row.getComponent(), row};
+		}
+
+		decorated.sort((a, b) => {
+			for(let i = sorterCount; i >= 0; i--){
+				const sortItem = sortMeta[i];
+				const asc = sortItem.asc;
+				const result = sortItem.sorter.call(this,
+					asc ? a.values[i] : b.values[i],
+					asc ? b.values[i] : a.values[i],
+					asc ? a.component : b.component,
+					asc ? b.component : a.component,
+					sortItem.columnComponent,
+					sortItem.dir,
+					sortItem.params
+				);
+
 				if(result !== 0){
-					break;
+					return result;
 				}
 			}
-			
-			return result;
+
+			return 0;
 		});
+
+		for(let k = 0; k < length; k++){
+			data[k] = decorated[k].row;
+		}
 	}
-
-	//process individual rows for a sort function on active data
-	_sortRow(a, b, column, dir, params){
-		var el1Comp, el2Comp;
-
-		//switch elements depending on search direction
-		var el1 = dir == "asc" ? a : b;
-		var el2 = dir == "asc" ? b : a;
-
-		a = column.getFieldValue(el1.getData());
-		b = column.getFieldValue(el2.getData());
-
-		a = typeof a !== "undefined" ? a : "";
-		b = typeof b !== "undefined" ? b : "";
-
-		el1Comp = el1.getComponent();
-		el2Comp = el2.getComponent();
-
-		return column.modules.sort.sorter.call(this, a, b, el1Comp, el2Comp, column.getComponent(), dir, params);
-	}
-}
+}

src/js/modules/Sort/defaults/sorters/number.js

@@ -1,40 +1,58 @@
 //sort numbers
 export default function(a, b, aRow, bRow, column, dir, params){
+	//fast-path: both already finite numbers, no separator/empty handling needed
+	if(typeof a === "number" && typeof b === "number" && isFinite(a) && isFinite(b)){
+		return a - b;
+	}
+
 	var alignEmptyValues = params.alignEmptyValues;
 	var decimal = params.decimalSeparator;
 	var thousand = params.thousandSeparator;
 	var emptyAlign = 0;
+	var aEmpty = a === "" || a === null || typeof a === "undefined";
+	var bEmpty = b === "" || b === null || typeof b === "undefined";
 
-	a = String(a);
-	b = String(b);
+	if(aEmpty){
+		emptyAlign = bEmpty ? 0 : -1;
+	}else if(bEmpty){
+		emptyAlign = 1;
+	}else{
+		a = parseValue(a, decimal, thousand);
+		b = parseValue(b, decimal, thousand);
 
-	if(thousand){
-		a = a.split(thousand).join("");
-		b = b.split(thousand).join("");
+		//handle non numeric values
+		if(isNaN(a)){
+			emptyAlign =  isNaN(b) ? 0 : -1;
+		}else if(isNaN(b)){
+			emptyAlign =  1;
+		}else{
+			//compare valid values
+			return a - b;
+		}
 	}
 
-	if(decimal){
-		a = a.split(decimal).join(".");
-		b = b.split(decimal).join(".");
+	//fix empty values in position
+	if((alignEmptyValues === "top" && dir === "desc") || (alignEmptyValues === "bottom" && dir === "asc")){
+		emptyAlign *= -1;
 	}
 
-	a = parseFloat(a);
-	b = parseFloat(b);
+	return emptyAlign;
+}
 
-	//handle non numeric values
-	if(isNaN(a)){
-		emptyAlign =  isNaN(b) ? 0 : -1;
-	}else if(isNaN(b)){
-		emptyAlign =  1;
-	}else{
-		//compare valid values
-		return a - b;
+function parseValue(value, decimal, thousand){
+	if(typeof value === "number"){
+		return value;
 	}
 
-	//fix empty values in position
-	if((alignEmptyValues === "top" && dir === "desc") || (alignEmptyValues === "bottom" && dir === "asc")){
-		emptyAlign *= -1;
+	value = String(value);
+
+	if(thousand){
+		value = value.replaceAll(thousand, "");
 	}
 
-	return emptyAlign;
-}
+	if(decimal && decimal !== "." ){
+		value = value.replaceAll(decimal, ".");
+	}
+
+	return parseFloat(value);
+}

test/unit/modules/SortItems.spec.js

@@ -0,0 +1,120 @@
+import TabulatorFull from "../../../src/js/core/TabulatorFull";
+
+// Correctness coverage for Sort._sortItems decorate-sort-undecorate rewrite.
+// Builds real rows/columns via TabulatorFull (the Sort.spec.js pattern) and
+// validates ordering + stability against a reference implementation of the
+// stock per-comparison algorithm, so the tests exercise the real Row/Column
+// contract rather than hand-built fakes.
+
+describe("Sort._sortItems (decorate-sort-undecorate)", () => {
+	/** @type {TabulatorFull} */
+	let tabulator;
+	let sortMod;
+
+	const columns = [
+		{ title: "A", field: "a", sorter: "number" },
+		{ title: "B", field: "b", sorter: "number" },
+		{ title: "Name", field: "name", sorter: "string" },
+		{ title: "Id", field: "id", sorter: "number" },
+	];
+
+	beforeEach(() => {
+		const el = document.createElement("div");
+		el.id = "tabulator";
+		document.body.appendChild(el);
+		tabulator = new TabulatorFull("#tabulator", { data: [], columns });
+		sortMod = tabulator.module("sort");
+		return new Promise((resolve) => tabulator.on("tableBuilt", resolve));
+	});
+
+	afterEach(() => {
+		tabulator.destroy();
+		document.getElementById("tabulator")?.remove();
+	});
+
+	// Reference: an independent per-comparison oracle (the pre-rewrite sort algorithm),
+	// run over the same real rows/columns _sortItems sees.
+	function referenceSort(rows, sortList) {
+		const sorterCount = sortList.length - 1;
+		return rows.slice().sort((a, b) => {
+			let result = 0;
+			for (let i = sorterCount; i >= 0; i--) {
+				const { column, dir } = sortList[i];
+				const { sorter, params } = column.modules.sort;
+				const el1 = dir === "asc" ? a : b;
+				const el2 = dir === "asc" ? b : a;
+				let av = column.getFieldValue(el1.getData());
+				let bv = column.getFieldValue(el2.getData());
+				av = typeof av !== "undefined" ? av : "";
+				bv = typeof bv !== "undefined" ? bv : "";
+				result = sorter.call(sortMod, av, bv, el1.getComponent(), el2.getComponent(), column.getComponent(), dir, params);
+				if (result !== 0) break;
+			}
+			return result;
+		});
+	}
+
+	// Load data, apply sort, and return { actual, expected, original } where actual is
+	// produced by Sort.sort() (which delegates to _sortItems) and expected by the reference.
+	async function sortData(data, sortSpec) {
+		await tabulator.setData(data);
+		const sortList = sortSpec.map(({ field, dir }) => ({ column: tabulator.columnManager.findColumn(field), dir }));
+		sortMod.setSort(sortList);
+		const original = tabulator.rowManager.activeRows.slice();
+		const actual = sortMod.sort(original.slice());
+		return { actual, expected: referenceSort(original, sortList) };
+	}
+
+	const ids = (rows) => rows.map((row) => row.data.id);
+	const field = (rows, key) => rows.map((row) => row.data[key]);
+
+	test("single numeric column ascending", async () => {
+		const { actual } = await sortData([{ id: 1, a: 3 }, { id: 2, a: 1 }, { id: 3, a: 2 }], [{ field: "a", dir: "asc" }]);
+		expect(field(actual, "a")).toEqual([1, 2, 3]);
+	});
+
+	test("single numeric column descending", async () => {
+		const { actual } = await sortData([{ id: 1, a: 3 }, { id: 2, a: 1 }, { id: 3, a: 2 }], [{ field: "a", dir: "desc" }]);
+		expect(field(actual, "a")).toEqual([3, 2, 1]);
+	});
+
+	test("sorts the passed data array in place (same reference)", async () => {
+		await tabulator.setData([{ id: 1, a: 2 }, { id: 2, a: 1 }]);
+		sortMod.setSort([{ column: tabulator.columnManager.findColumn("a"), dir: "asc" }]);
+		const rows = tabulator.rowManager.activeRows.slice();
+		const ref = rows;
+		const out = sortMod.sort(rows);
+		expect(out).toBe(ref);
+		expect(field(rows, "a")).toEqual([1, 2]);
+	});
+
+	test("multi-column ordering matches reference", async () => {
+		const data = [
+			{ id: 1, a: 2, b: 1 }, { id: 2, a: 1, b: 2 }, { id: 3, a: 1, b: 1 }, { id: 4, a: 2, b: 2 },
+		];
+		const { actual, expected } = await sortData(data, [{ field: "b", dir: "asc" }, { field: "a", dir: "asc" }]);
+		expect(ids(actual)).toEqual(ids(expected));
+	});
+
+	test("stability: equal keys preserve original order", async () => {
+		const { actual } = await sortData([{ id: 1, a: 1 }, { id: 2, a: 1 }, { id: 3, a: 1 }], [{ field: "a", dir: "asc" }]);
+		expect(ids(actual)).toEqual([1, 2, 3]);
+	});
+
+	test("matches reference over randomized multi-column (numeric + string, asc + desc)", async () => {
+		const sortSpec = [{ field: "name", dir: "desc" }, { field: "a", dir: "asc" }];
+		let seed = 99;
+		const rnd = () => { seed = (seed * 1103515245 + 12345) & 0x7fffffff; return seed / 0x7fffffff; };
+		for (let trial = 0; trial < 10; trial++) {
+			const data = [];
+			for (let i = 0; i < 60; i++) data.push({ id: i, a: Math.floor(rnd() * 5), name: "n" + Math.floor(rnd() * 5) });
+			const { actual, expected } = await sortData(data, sortSpec);
+			expect(ids(actual)).toEqual(ids(expected));
+		}
+	});
+
+	test("undefined field values are coerced to empty string (as stock)", async () => {
+		const { actual, expected } = await sortData([{ id: 1, a: 5 }, { id: 2 }, { id: 3, a: 2 }], [{ field: "a", dir: "asc" }]);
+		expect(ids(actual)).toEqual(ids(expected));
+	});
+});