Skip to content

Commit 3e8d625

Browse files
committed
[PERF] coordinate: faster xc parsing
This commit improves the parsing of references by removing regular expressions, string allocations, splits, and concatenations, which were heavily used in the previous implementation. Total time spent in `toCartesian` when loading the BE Timesheet dashboard: before: 84ms after: 27ms (3x faster!) Task: 4771804 X-original-commit: 24e0b5a Part-of: #6429 Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com> Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
1 parent 52824ed commit 3e8d625

File tree

2 files changed

+54
-28
lines changed

2 files changed

+54
-28
lines changed

src/helpers/coordinates.ts

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//------------------------------------------------------------------------------
44

55
import { HeaderIndex, Position, RangePart } from "../types";
6+
import { TokenizingChars } from "./misc";
67

78
/**
89
* Convert a (col) number to the corresponding letter.
@@ -28,13 +29,17 @@ export function lettersToNumber(letters: string): number {
2829
let result = 0;
2930
const l = letters.length;
3031
for (let i = 0; i < l; i++) {
31-
const charCode = letters.charCodeAt(i);
32-
const colIndex = charCode >= 65 && charCode <= 90 ? charCode - 64 : charCode - 96;
32+
const colIndex = charToNumber(letters[i]);
3333
result = result * 26 + colIndex;
3434
}
3535
return result - 1;
3636
}
3737

38+
function charToNumber(char: string) {
39+
const charCode = char.charCodeAt(0);
40+
return charCode >= 65 && charCode <= 90 ? charCode - 64 : charCode - 96;
41+
}
42+
3843
function isCharALetter(char: string) {
3944
return (char >= "A" && char <= "Z") || (char >= "a" && char <= "z");
4045
}
@@ -43,6 +48,40 @@ function isCharADigit(char: string) {
4348
return char >= "0" && char <= "9";
4449
}
4550

51+
// we limit the max column to 3 letters and max row to 7 digits for performance reasons
52+
export const MAX_COL = lettersToNumber("ZZZ");
53+
export const MAX_ROW = 9999998;
54+
55+
export function consumeSpaces(chars: TokenizingChars) {
56+
while (chars.current === " ") {
57+
chars.advanceBy(1);
58+
}
59+
}
60+
61+
export function consumeLetters(chars: TokenizingChars): number {
62+
if (chars.current === "$") chars.advanceBy(1);
63+
if (!chars.current || !isCharALetter(chars.current)) {
64+
return -1;
65+
}
66+
let colCoordinate = 0;
67+
while (chars.current && isCharALetter(chars.current)) {
68+
colCoordinate = colCoordinate * 26 + charToNumber(chars.shift());
69+
}
70+
return colCoordinate;
71+
}
72+
73+
export function consumeDigits(chars: TokenizingChars): number {
74+
if (chars.current === "$") chars.advanceBy(1);
75+
if (!chars.current || !isCharADigit(chars.current)) {
76+
return -1;
77+
}
78+
let num = 0;
79+
while (chars.current && isCharADigit(chars.current)) {
80+
num = num * 10 + Number(chars.shift());
81+
}
82+
return num;
83+
}
84+
4685
/**
4786
* Convert a "XC" coordinate to cartesian coordinates.
4887
*
@@ -53,37 +92,21 @@ function isCharADigit(char: string) {
5392
* Note: it also accepts lowercase coordinates, but not fixed references
5493
*/
5594
export function toCartesian(xc: string): Position {
56-
xc = xc.trim();
57-
58-
let letterPart = "";
59-
let numberPart = "";
60-
let i = 0;
95+
const chars = new TokenizingChars(xc);
6196

62-
// Process letter part
63-
if (xc[i] === "$") i++;
64-
while (i < xc.length && isCharALetter(xc[i])) {
65-
letterPart += xc[i++];
66-
}
97+
consumeSpaces(chars);
98+
const letterPart = consumeLetters(chars);
6799

68-
if (letterPart.length === 0 || letterPart.length > 3) {
69-
// limit to max 3 letters for performance reasons
100+
if (letterPart === -1 || !chars.current) {
70101
throw new Error(`Invalid cell description: ${xc}`);
71102
}
72103

73-
// Process number part
74-
if (xc[i] === "$") i++;
75-
while (i < xc.length && isCharADigit(xc[i])) {
76-
numberPart += xc[i++];
77-
}
78-
79-
if (i !== xc.length || numberPart.length === 0 || numberPart.length > 7) {
80-
// limit to max 7 numbers for performance reasons
81-
throw new Error(`Invalid cell description: ${xc}`);
82-
}
104+
const num = consumeDigits(chars);
105+
consumeSpaces(chars);
83106

84-
const col = lettersToNumber(letterPart);
85-
const row = Number(numberPart) - 1;
86-
if (isNaN(row)) {
107+
const col = letterPart - 1;
108+
const row = num - 1;
109+
if (!chars.isOver() || col > MAX_COL || row > MAX_ROW) {
87110
throw new Error(`Invalid cell description: ${xc}`);
88111
}
89112
return { col, row };

tests/helpers/coordinates_helpers.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@ describe("numberToLetter", () => {
1717
describe("toCartesian", () => {
1818
test("basic functionality", () => {
1919
expect(toCartesian("A1")).toEqual(toPosition(0, 0));
20+
expect(toCartesian("A01")).toEqual(toPosition(0, 0));
2021
expect(toCartesian("B1")).toEqual(toPosition(1, 0));
2122
expect(toCartesian("C5")).toEqual(toPosition(2, 4));
2223
expect(toCartesian("AA55")).toEqual(toPosition(26, 54));
2324
expect(toCartesian("c5")).toEqual(toPosition(2, 4));
2425
expect(toCartesian(" C5 ")).toEqual(toPosition(2, 4));
2526
expect(toCartesian("AAA1")).toEqual(toPosition(702, 0));
26-
expect(toCartesian("A999999")).toEqual(toPosition(0, 999998));
27+
expect(toCartesian("ZZZ1")).toEqual(toPosition(18277, 0));
28+
expect(toCartesian("A9999999")).toEqual(toPosition(0, 9999998));
2729
});
2830

2931
test("invalid ranges", () => {
@@ -35,6 +37,7 @@ describe("toCartesian", () => {
3537
expect(() => toCartesian("")).toThrow("Invalid cell description: ");
3638
expect(() => toCartesian(" ")).toThrow("Invalid cell description: ");
3739
expect(() => toCartesian("AAAA1")).toThrow("Invalid cell description: AAAA1");
40+
expect(() => toCartesian("ZZZA1")).toThrow("Invalid cell description: ZZZA1");
3841
expect(() => toCartesian("A10000000")).toThrow("Invalid cell description: A10000000");
3942
});
4043
});

0 commit comments

Comments
 (0)