From 3ad2d9212e09fb7ec0a351f334d789036e547c47 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Mon, 23 May 2022 18:34:15 +0200 Subject: [PATCH] (core) Prevent rebuilding usercode for every AddColumn when importing Summary: Importing a file with many columns would be very slow due to expensive calls to rebuild_usercode for each added column: https://grist.slack.com/archives/C02EGJ1FUCV/p1652395747972749?thread_ts=1652388644.394419&cid=C02EGJ1FUCV This diff suppresses rebuild_usercode temporarily while adding columns in a loop in MakeImportTransformColumns, then calls it once afterwards. Test Plan: Manually imported a wide file repeatedly. Eventually, whehn importing a file with 300 columns, generating the preview went from taking about 100 seconds to 20 seconds. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3445 --- sandbox/grist/engine.py | 8 +++++++ sandbox/grist/import_actions.py | 39 +++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/sandbox/grist/engine.py b/sandbox/grist/engine.py index 3338c09f..e8efd599 100644 --- a/sandbox/grist/engine.py +++ b/sandbox/grist/engine.py @@ -213,6 +213,10 @@ class Engine(object): # A flag for when a useraction causes a schema change, to verify consistency afterwards. self._schema_updated = False + # Set to false temporarily to suppress rebuild_usercode for performance. + # Used when importing which can add many columns which calls rebuild_usercode each time. + self._should_rebuild_usercode = True + # Stores an exception representing the first unevaluated cell met while recomputing the # current cell. self._cell_required_error = None @@ -1013,6 +1017,9 @@ class Engine(object): """ Compiles the usercode from the schema, and updates all tables and columns to match. """ + if not self._should_rebuild_usercode: + return + self.gencode.make_module(self.schema) # Re-populate self.tables, reusing existing tables whenever possible. @@ -1281,6 +1288,7 @@ class Engine(object): if saved_schema: log.info("Restoring schema and usercode on exception") self.schema = saved_schema + self._should_rebuild_usercode = True try: self.rebuild_usercode() except Exception: diff --git a/sandbox/grist/import_actions.py b/sandbox/grist/import_actions.py index 753f7bde..ad0eb945 100644 --- a/sandbox/grist/import_actions.py +++ b/sandbox/grist/import_actions.py @@ -147,26 +147,33 @@ class ImportActions(object): log.debug("MakeImportTransformColumns: {}".format("gen_all" if gen_all else "optimize")) + # Calling rebuild_usercode once per added column is wasteful and can be very slow. + self._engine._should_rebuild_usercode = False + #create prefixed formula column for each of dest_cols #take formula from transform_rule new_cols = [] - for c in dest_cols: - # skip copy columns (unless gen_all) - formula = c.formula.strip() - isCopyFormula = (formula.startswith("$") and formula[1:] in src_cols) + try: + for c in dest_cols: + # skip copy columns (unless gen_all) + formula = c.formula.strip() + isCopyFormula = (formula.startswith("$") and formula[1:] in src_cols) - if gen_all or not isCopyFormula: - # If colId specified, use that. Otherwise, use the (sanitized) label. - col_id = c.colId or identifiers.pick_col_ident(c.label) - new_col_id = _import_transform_col_prefix + col_id - new_col_spec = { - "label": c.label, - "type": c.type, - "widgetOptions": getattr(c, "widgetOptions", ""), - "isFormula": True, - "formula": c.formula} - result = self._useractions.doAddColumn(hidden_table_id, new_col_id, new_col_spec) - new_cols.append(result["colRef"]) + if gen_all or not isCopyFormula: + # If colId specified, use that. Otherwise, use the (sanitized) label. + col_id = c.colId or identifiers.pick_col_ident(c.label) + new_col_id = _import_transform_col_prefix + col_id + new_col_spec = { + "label": c.label, + "type": c.type, + "widgetOptions": getattr(c, "widgetOptions", ""), + "isFormula": True, + "formula": c.formula} + result = self._useractions.doAddColumn(hidden_table_id, new_col_id, new_col_spec) + new_cols.append(result["colRef"]) + finally: + self._engine._should_rebuild_usercode = True + self._engine.rebuild_usercode() return new_cols