(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
This commit is contained in:
Alex Hall 2022-05-23 18:34:15 +02:00
parent 6793377579
commit 3ad2d9212e
2 changed files with 31 additions and 16 deletions

View File

@ -213,6 +213,10 @@ class Engine(object):
# A flag for when a useraction causes a schema change, to verify consistency afterwards. # A flag for when a useraction causes a schema change, to verify consistency afterwards.
self._schema_updated = False 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 # Stores an exception representing the first unevaluated cell met while recomputing the
# current cell. # current cell.
self._cell_required_error = None 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. 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) self.gencode.make_module(self.schema)
# Re-populate self.tables, reusing existing tables whenever possible. # Re-populate self.tables, reusing existing tables whenever possible.
@ -1281,6 +1288,7 @@ class Engine(object):
if saved_schema: if saved_schema:
log.info("Restoring schema and usercode on exception") log.info("Restoring schema and usercode on exception")
self.schema = saved_schema self.schema = saved_schema
self._should_rebuild_usercode = True
try: try:
self.rebuild_usercode() self.rebuild_usercode()
except Exception: except Exception:

View File

@ -147,26 +147,33 @@ class ImportActions(object):
log.debug("MakeImportTransformColumns: {}".format("gen_all" if gen_all else "optimize")) 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 #create prefixed formula column for each of dest_cols
#take formula from transform_rule #take formula from transform_rule
new_cols = [] new_cols = []
for c in dest_cols: try:
# skip copy columns (unless gen_all) for c in dest_cols:
formula = c.formula.strip() # skip copy columns (unless gen_all)
isCopyFormula = (formula.startswith("$") and formula[1:] in src_cols) formula = c.formula.strip()
isCopyFormula = (formula.startswith("$") and formula[1:] in src_cols)
if gen_all or not isCopyFormula: if gen_all or not isCopyFormula:
# If colId specified, use that. Otherwise, use the (sanitized) label. # If colId specified, use that. Otherwise, use the (sanitized) label.
col_id = c.colId or identifiers.pick_col_ident(c.label) col_id = c.colId or identifiers.pick_col_ident(c.label)
new_col_id = _import_transform_col_prefix + col_id new_col_id = _import_transform_col_prefix + col_id
new_col_spec = { new_col_spec = {
"label": c.label, "label": c.label,
"type": c.type, "type": c.type,
"widgetOptions": getattr(c, "widgetOptions", ""), "widgetOptions": getattr(c, "widgetOptions", ""),
"isFormula": True, "isFormula": True,
"formula": c.formula} "formula": c.formula}
result = self._useractions.doAddColumn(hidden_table_id, new_col_id, new_col_spec) result = self._useractions.doAddColumn(hidden_table_id, new_col_id, new_col_spec)
new_cols.append(result["colRef"]) new_cols.append(result["colRef"])
finally:
self._engine._should_rebuild_usercode = True
self._engine.rebuild_usercode()
return new_cols return new_cols