Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0] Various easy performance boosts #2340

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions demo/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -1514,12 +1514,22 @@ function computeFormulaCells(cols, rows) {
return cells;
}

function computeNumberCells(cols, rows) {
function computeNumberCells(cols, rows, type = "numbers") {
const cells = {};
for (let col = 0; col < cols; col++) {
const letter = _getColumnLetter(col);
for (let index = 1; index < rows - 1; index++) {
cells[letter + index] = { content: `${col + index}` };
switch (type) {
case "numbers":
cells[letter + index] = { content: `${col + index}` };
break;
case "floats":
cells[letter + index] = { content: `${col + index}.123` };
break;
case "longFloats":
cells[letter + index] = { content: `${col + index}.123456789123456` };
break;
}
}
}
return cells;
Expand Down Expand Up @@ -1552,7 +1562,9 @@ export function makeLargeDataset(cols, rows, sheetsInfo = ["formulas"]) {
cells = computeFormulaCells(cols, rows);
break;
case "numbers":
cells = computeNumberCells(cols, rows);
case "floats":
case "longFloats":
cells = computeNumberCells(cols, rows, sheetsInfo[0]);
break;
case "strings":
cells = computeStringCells(cols, rows);
Expand Down
8 changes: 5 additions & 3 deletions src/components/grid/grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
HEADER_WIDTH,
SCROLLBAR_WIDTH,
} from "../../constants";
import { isInside, range } from "../../helpers/index";
import { isInside } from "../../helpers/index";
import { interactiveCut } from "../../helpers/ui/cut_interactive";
import { interactivePaste, interactivePasteFromOS } from "../../helpers/ui/paste_interactive";
import { ComposerSelection } from "../../plugins/ui/edition";
Expand Down Expand Up @@ -224,12 +224,14 @@ export class Grid extends Component<Props, SpreadsheetChildEnv> {
const col = this.env.model.getters.findVisibleHeader(
sheetId,
"COL",
range(0, this.env.model.getters.getNumberCols(sheetId)).reverse()
this.env.model.getters.getNumberCols(sheetId) - 1,
0
)!;
const row = this.env.model.getters.findVisibleHeader(
sheetId,
"ROW",
range(0, this.env.model.getters.getNumberRows(sheetId)).reverse()
this.env.model.getters.getNumberRows(sheetId) - 1,
0
)!;
this.env.model.selection.selectCell(col, row);
},
Expand Down
92 changes: 89 additions & 3 deletions src/helpers/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,100 @@ const numberRepresentation: Intl.NumberFormat[] = [];
* - all digit stored in the decimal part of the number
*
* The 'maxDecimal' parameter allows to indicate the number of digits to not
* exceed in the decimal part, in which case digits are rounded
* exceed in the decimal part, in which case digits are rounded.
*
* Intl.Numberformat is used to properly handle all the roundings.
* e.g. 1234.7 with format ### (<> maxDecimals=0) should become 1235, not 1234
**/
function splitNumber(
value: number,
maxDecimals: number = MAX_DECIMAL_PLACES
): { integerDigits: string; decimalDigits: string | undefined } {
const asString = value.toString();
if (asString.includes("e")) return splitNumberIntl(value, maxDecimals);
hokolomopo marked this conversation as resolved.
Show resolved Hide resolved

if (Number.isInteger(value)) {
return { integerDigits: asString, decimalDigits: undefined };
}

const indexOfDot = asString.indexOf(".");
let integerDigits = asString.substring(0, indexOfDot);
let decimalDigits: string | undefined = asString.substring(indexOfDot + 1);

if (maxDecimals === 0) {
if (Number(decimalDigits[0]) >= 5) {
integerDigits = (Number(integerDigits) + 1).toString();
}
return { integerDigits, decimalDigits: undefined };
}

if (decimalDigits.length > maxDecimals) {
const { integerDigits: roundedIntegerDigits, decimalDigits: roundedDecimalDigits } =
limitDecimalDigits(decimalDigits, maxDecimals);

decimalDigits = roundedDecimalDigits;
if (roundedIntegerDigits !== "0") {
integerDigits = (Number(integerDigits) + Number(roundedIntegerDigits)).toString();
}
}

return { integerDigits, decimalDigits: removeTrailingZeroes(decimalDigits || "") };
}

/**
* Return the given string minus the trailing "0" characters.
*
* @param numberString : a string of integers
* @returns the numberString, minus the eventual zeroes at the end
*/
function removeTrailingZeroes(numberString: string): string | undefined {
let i = numberString.length - 1;
while (i >= 0 && numberString[i] === "0") {
i--;
}
return numberString.slice(0, i + 1) || undefined;
}

/**
* Limit the size of the decimal part of a number to the given number of digits.
*/
function limitDecimalDigits(
decimalDigits: string,
maxDecimals: number
): {
integerDigits: string;
decimalDigits: string | undefined;
} {
let integerDigits = "0";
let resultDecimalDigits: string | undefined = decimalDigits;

// Note : we'd want to simply use number.toFixed() to handle the max digits & rounding,
// but it has very strange behaviour. Ex: 12.345.toFixed(2) => "12.35", but 1.345.toFixed(2) => "1.34"
let slicedDecimalDigits = decimalDigits.slice(0, maxDecimals);
const i = maxDecimals;

if (Number(Number(decimalDigits[i]) < 5)) {
return { integerDigits, decimalDigits: slicedDecimalDigits };
}

// round up
const slicedRoundedUp = (Number(slicedDecimalDigits) + 1).toString();
if (slicedRoundedUp.length > slicedDecimalDigits.length) {
integerDigits = (Number(integerDigits) + 1).toString();
resultDecimalDigits = undefined;
} else {
resultDecimalDigits = slicedRoundedUp;
}

return { integerDigits, decimalDigits: resultDecimalDigits };
}

/**
* Split numbers into decimal/integer digits using Intl.NumberFormat.
* Supports numbers with a lot of digits that are transformed to scientific notation by
* number.toString(), but is slow.
*/
function splitNumberIntl(
value: number,
maxDecimals: number = MAX_DECIMAL_PLACES
): { integerDigits: string; decimalDigits: string | undefined } {
let formatter = numberRepresentation[maxDecimals];
if (!formatter) {
Expand Down
4 changes: 2 additions & 2 deletions src/migrations/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
FORBIDDEN_IN_EXCEL_REGEX,
FORMULA_REF_IDENTIFIER,
} from "../constants";
import { getItemId, toXC, toZone, UuidGenerator } from "../helpers/index";
import { deepCopy, getItemId, toXC, toZone, UuidGenerator } from "../helpers/index";
import { StateUpdateMessage } from "../types/collaborative/transport_service";
import {
CoreCommand,
Expand Down Expand Up @@ -46,7 +46,7 @@ export function load(data?: any, verboseImport?: boolean): WorkbookData {
}
}
}
data = JSON.parse(JSON.stringify(data));
data = deepCopy(data);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not imported in the file :o


// apply migrations, if needed
if ("version" in data) {
Expand Down
23 changes: 15 additions & 8 deletions src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ export class Model extends EventBus<any> implements CommandDispatcher {

uuidGenerator: UuidGenerator;

private readonly handlers: CommandHandler<Command>[] = [];
private readonly coreHandlers: CommandHandler<CoreCommand>[] = [];

constructor(
data: any = {},
config: Partial<ModelConfig> = {},
Expand Down Expand Up @@ -194,6 +197,9 @@ export class Model extends EventBus<any> implements CommandDispatcher {
// Initiate stream processor
this.selection = new SelectionStreamProcessor(this.getters);

this.coreHandlers.push(this.range);
this.handlers.push(this.range);

// registering plugins
for (let Plugin of corePluginRegistry.getAll()) {
this.setupCorePlugin(Plugin, workbookData);
Expand All @@ -204,6 +210,8 @@ export class Model extends EventBus<any> implements CommandDispatcher {
}
this.uuidGenerator.setIsFastStrategy(false);

this.handlers.push(this.history);

// starting plugins
this.dispatch("START");
// Model should be the last permanent subscriber in the list since he should render
Expand All @@ -227,10 +235,6 @@ export class Model extends EventBus<any> implements CommandDispatcher {
markRaw(this);
}

get handlers(): CommandHandler<Command>[] {
return [this.range, ...this.corePlugins, ...this.uiPlugins, this.history];
}

joinSession() {
this.session.join(this.config.client);
}
Expand All @@ -251,6 +255,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
this.getters[name] = plugin[name].bind(plugin);
}
this.uiPlugins.push(plugin);
this.handlers.push(plugin);
const layers = Plugin.layers.map((l) => [plugin, l] as [UIPlugin, LAYERS]);
this.renderers.push(...layers);
this.renderers.sort((p1, p2) => p1[1] - p2[1]);
Expand Down Expand Up @@ -282,6 +287,8 @@ export class Model extends EventBus<any> implements CommandDispatcher {
}
plugin.import(data);
this.corePlugins.push(plugin);
this.coreHandlers.push(plugin);
this.handlers.push(plugin);
}

private onRemoteRevisionReceived({ commands }: { commands: CoreCommand[] }) {
Expand All @@ -305,7 +312,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
return;
}
this.isReplayingCommand = true;
this.dispatchToHandlers([this.range, ...this.corePlugins], command);
this.dispatchToHandlers(this.coreHandlers, command);
this.isReplayingCommand = false;
}
),
Expand Down Expand Up @@ -437,7 +444,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
const command: Command = { type, ...payload };
const previousStatus = this.status;
this.status = Status.RunningCore;
const handlers = this.isReplayingCommand ? [this.range, ...this.corePlugins] : this.handlers;
const handlers = this.isReplayingCommand ? this.coreHandlers : this.handlers;
this.dispatchToHandlers(handlers, command);
this.status = previousStatus;
return DispatchResult.Success;
Expand Down Expand Up @@ -497,7 +504,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
}
}
data.revisionId = this.session.getRevisionId() || DEFAULT_REVISION_ID;
data = JSON.parse(JSON.stringify(data));
data = deepCopy(data);
return data;
}

Expand Down Expand Up @@ -526,7 +533,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
handler.exportForExcel(data);
}
}
data = JSON.parse(JSON.stringify(data));
data = deepCopy(data);

return getXLSX(data);
}
Expand Down
25 changes: 13 additions & 12 deletions src/plugins/core/sheet.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { FORBIDDEN_IN_EXCEL_REGEX } from "../../constants";
import {
createDefaultRows,
deepCopy,
getUnquotedSheetName,
groupConsecutive,
isDefined,
Expand Down Expand Up @@ -686,7 +687,7 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
private duplicateSheet(fromId: UID, toId: UID) {
const sheet = this.getSheet(fromId);
const toName = this.getDuplicateSheetName(sheet.name);
const newSheet: Sheet = JSON.parse(JSON.stringify(sheet));
const newSheet: Sheet = deepCopy(sheet);
newSheet.id = toId;
newSheet.name = toName;
for (let col = 0; col <= newSheet.numberOfCols; col++) {
Expand Down Expand Up @@ -834,10 +835,10 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
}

private moveCellOnColumnsDeletion(sheet: Sheet, deletedColumn: number) {
for (let [index, row] of Object.entries(sheet.rows)) {
const rowIndex = parseInt(index, 10);
hokolomopo marked this conversation as resolved.
Show resolved Hide resolved
for (let rowIndex = 0; rowIndex < sheet.rows.length; rowIndex++) {
const row = sheet.rows[rowIndex];
for (let i in row.cells) {
const colIndex = parseInt(i, 10);
const colIndex = Number(i);
const cellId = row.cells[i];
if (cellId) {
if (colIndex === deletedColumn) {
Expand Down Expand Up @@ -870,11 +871,11 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
dimension: "rows" | "columns"
) {
const commands: UpdateCellPositionCommand[] = [];
for (const [index, row] of Object.entries(sheet.rows)) {
const rowIndex = parseInt(index, 10);
for (let rowIndex = 0; rowIndex < sheet.rows.length; rowIndex++) {
const row = sheet.rows[rowIndex];
if (dimension !== "rows" || rowIndex >= addedElement) {
for (let i in row.cells) {
const colIndex = parseInt(i, 10);
const colIndex = Number(i);
const cellId = row.cells[i];
if (cellId) {
if (dimension === "rows" || colIndex >= addedElement) {
Expand Down Expand Up @@ -909,11 +910,11 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
deleteToRow: HeaderIndex
) {
const numberRows = deleteToRow - deleteFromRow + 1;
for (let [index, row] of Object.entries(sheet.rows)) {
const rowIndex = parseInt(index, 10);
for (let rowIndex = 0; rowIndex < sheet.rows.length; rowIndex++) {
const row = sheet.rows[rowIndex];
if (rowIndex >= deleteFromRow && rowIndex <= deleteToRow) {
for (let i in row.cells) {
const colIndex = parseInt(i, 10);
const colIndex = Number(i);
const cellId = row.cells[i];
if (cellId) {
this.dispatch("CLEAR_CELL", {
Expand All @@ -926,7 +927,7 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
}
if (rowIndex > deleteToRow) {
for (let i in row.cells) {
const colIndex = parseInt(i, 10);
const colIndex = Number(i);
const cellId = row.cells[i];
if (cellId) {
this.dispatch("UPDATE_CELL_POSITION", {
Expand All @@ -945,7 +946,7 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
const rows: Row[] = [];
const cellsQueue = sheet.rows.map((row) => row.cells);
for (let i in sheet.rows) {
if (parseInt(i, 10) === index) {
if (Number(i) === index) {
continue;
}
rows.push({
Expand Down
10 changes: 4 additions & 6 deletions src/plugins/ui/filter_evaluation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,11 @@ export class FilterEvaluationPlugin extends UIPlugin {
}

private intersectZoneWithViewport(sheetId: UID, zone: Zone) {
const colsRange = range(zone.left, zone.right + 1);
const rowsRange = range(zone.top, zone.bottom + 1);
return {
left: this.getters.findVisibleHeader(sheetId, "COL", colsRange),
right: this.getters.findVisibleHeader(sheetId, "COL", colsRange.reverse()),
top: this.getters.findVisibleHeader(sheetId, "ROW", rowsRange),
bottom: this.getters.findVisibleHeader(sheetId, "ROW", rowsRange.reverse()),
left: this.getters.findVisibleHeader(sheetId, "COL", zone.left, zone.right),
right: this.getters.findVisibleHeader(sheetId, "COL", zone.right, zone.left),
top: this.getters.findVisibleHeader(sheetId, "ROW", zone.top, zone.bottom),
bottom: this.getters.findVisibleHeader(sheetId, "ROW", zone.bottom, zone.top),
};
}

Expand Down
Loading