(superset) 14/23: perf(activity-view): batch _datasets_used_by_chart for dashboard scope

[kg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kgabryje pushed a commit to branch sc-107283-versioning-activity-view
in repository https://gitbox.apache.org/repos/asf/superset.git

commit a2d5bd05d05728adf40661bc105002835306dd8b
Author: Mike Bridge <mi...@ext.preset.io>
AuthorDate: Tue May 26 16:29:52 2026 -0600

    perf(activity-view): batch _datasets_used_by_chart for dashboard scope
    
    Surfaced by T045's perf test (which failed at p95 4228ms vs 1500ms
    budget) and an ad-hoc per-phase profile that pinpointed _resolve_scope
    as accounting for ~1926ms of the wall clock.
    
    The previous _resolve_dashboard_scope called _datasets_used_by_chart
    once per slice on the dashboard. On a fixture with ~12 charts that's
    12 sequential SQL round-trips for the same shape of query, each
    returning a small result set. The new _batch_datasets_used_by_charts
    takes the full set of slice_ids and returns
    {slice_id: [(dataset_id, window), ...]} in one query; the singular
    form _datasets_used_by_chart now forwards to the batch (used only by
    _resolve_chart_scope which has a single slice).
    
    Impact (measured on the same fixture):
    * Wall-clock per request: 3955ms → 2257ms (43% faster)
    * Total query count per request: 18,376 → 29 (driven by the
      identity-map populating during the warmup; the genuine count for
      the activity layer alone is ~13 SQL queries)
    * T045 p95 over 50 runs: 4228ms → 2429ms
    
    The remaining gap to the 1500ms budget is dominated by Python work
    inside _fetch_change_records: post-filter and sort over ~26K
    accumulated test-environment records. The spec's target scale is
    "25 charts × 3 dataset windows" — the SQLite test environment has
    accumulated far more state from prior test runs. T046 (index
    migration) is deferred because the remaining bottleneck is Python-
    side, not SQL-side; production data on Postgres with retention
    pruning will not have this shape. Document and revisit when a
    production-representative perf measurement is available.
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <no...@anthropic.com>
---
 superset/versioning/activity.py | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/superset/versioning/activity.py b/superset/versioning/activity.py
index 76a5af49773..fb673189e5c 100644
--- a/superset/versioning/activity.py
+++ b/superset/versioning/activity.py
@@ -185,11 +185,33 @@ def _datasets_used_by_chart(slice_id: int) -> list[tuple[int, Window]]:
     """Return ``(datasource_id, window)`` for every dataset that *slice_id*
     has ever pointed at, with each association's validity window.
 
+    Single-slice form, used by :func:`_resolve_chart_scope` where there
+    is only one chart to walk. The dashboard-scope path calls
+    :func:`_batch_datasets_used_by_charts` instead so the query fires
+    once for all slices on the dashboard, not once per slice.
+
     Reads from ``slices_version`` (the chart parent shadow). Filters to
     ``datasource_type = 'table'`` because the activity view only follows
     the chart → ``SqlaTable`` dependency edge (not legacy/other
     datasources). Rows with ``operation_type = 2`` are excluded.
     """
+    return _batch_datasets_used_by_charts({slice_id}).get(slice_id, [])
+
+
+def _batch_datasets_used_by_charts(
+    slice_ids: set[int],
+) -> dict[int, list[tuple[int, Window]]]:
+    """Batch form of :func:`_datasets_used_by_chart`. Returns
+    ``{slice_id: [(dataset_id, window), ...]}`` in a single query so the
+    dashboard-scope walker doesn't fire one query per chart on the
+    dashboard. The previous per-slice shape became O(n_charts) round-
+    trips, which dominated ``get_activity`` latency on dashboards with
+    rich history (profile run 2026-05-26 showed `_resolve_scope`
+    accounting for ~1.9s out of 4s p95).
+    """
+    if not slice_ids:
+        return {}
+
     # pylint: disable=import-outside-toplevel
     from sqlalchemy_continuum import version_class
 
@@ -200,11 +222,12 @@ def _datasets_used_by_chart(slice_id: int) -> list[tuple[int, Window]]:
         db.session.connection()
         .execute(
             sa.select(
+                slices_tbl.c.id,
                 slices_tbl.c.datasource_id,
                 slices_tbl.c.transaction_id,
                 slices_tbl.c.end_transaction_id,
             ).where(
-                slices_tbl.c.id == slice_id,
+                slices_tbl.c.id.in_(slice_ids),
                 slices_tbl.c.datasource_type == "table",
                 slices_tbl.c.operation_type != 2,
                 slices_tbl.c.datasource_id.is_not(None),
@@ -212,7 +235,10 @@ def _datasets_used_by_chart(slice_id: int) -> list[tuple[int, Window]]:
         )
         .all()
     )
-    return [(row[0], (row[1], row[2])) for row in rows]
+    grouped: dict[int, list[tuple[int, Window]]] = {}
+    for slice_id, dataset_id, start_tx, end_tx in rows:
+        grouped.setdefault(slice_id, []).append((dataset_id, (start_tx, end_tx)))
+    return grouped
 
 
 # ---- T007: Window intersection (pure) -------------------------------------
@@ -1141,11 +1167,14 @@ def _resolve_dashboard_scope(dashboard_id: int) -> list[EntityWindows]:
     for slice_id, window in _charts_attached_to_dashboard(dashboard_id):
         chart_windows.setdefault(slice_id, []).append(window)
 
+    # One query for the dataset-history of every chart on the dashboard,
+    # not one query per chart. The per-slice form was O(n_charts) round-
+    # trips which dominated p95 on rich dashboards.
+    dataset_windows_by_slice = _batch_datasets_used_by_charts(set(chart_windows))
+
     for slice_id, attachment_windows in chart_windows.items():
         scope.append(("Slice", slice_id, list(attachment_windows)))
-        # Fetch the chart's dataset history once per slice — it doesn't
-        # depend on which attachment window we're clipping against.
-        dataset_windows = _datasets_used_by_chart(slice_id)
+        dataset_windows = dataset_windows_by_slice.get(slice_id, [])
         for attachment in attachment_windows:
             for dataset_id, chart_dataset_window in dataset_windows:
                 if (