Yicong-Huang commented on code in PR #6213:
URL: https://github.com/apache/texera/pull/6213#discussion_r3887539799
##########
frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts:
##########
@@ -108,6 +109,11 @@ export class JointGraphWrapper {
// reapply it to the shared model (covering both the main canvas and the
mini-map).
private regionsDisplayedStream = new BehaviorSubject<boolean>(false);
+ // The active performance heat-map view, or null when the overlay is off
(Layers > Performance).
+ // Kept here so the editor can (re)apply operator colors on the shared
model, covering both the
+ // main canvas and the mini-map.
+ private heatmapViewStream = new BehaviorSubject<HeatmapView | null>(null);
Review Comment:
Nothing ever resets this stream. `setHeatmapView` has two callers, both in
`MenuComponent`, which is per-workspace and re-initialises `showHeatmap =
false` on every entry. The wrapper itself is built once, by the root-provided
`WorkflowActionService`.
So leave Performance on, return to the dashboard, and open any workflow.
Teardown already cleared the metrics, so every operator repaints in the no-data
grey and the legend shows "—" — with the checkbox reading unchecked. Clicking
it then re-publishes the view rather than clearing it.
Regions has the same retention but draws nothing on re-entry, since its
elements only exist after a run.
I would reset the view in `WorkflowEditorComponent.ngOnDestroy`: the overlay
belongs to the canvas being viewed, and the metrics behind it are cleared at
that same moment.
##########
frontend/src/app/workspace/component/heatmap-legend/heatmap-legend.component.ts:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Component } from "@angular/core";
+import { AsyncPipe, NgIf } from "@angular/common";
+import { combineLatest, Observable } from "rxjs";
+import { map } from "rxjs/operators";
+import { WorkflowActionService } from
"../../service/workflow-graph/model/workflow-action.service";
+import { WorkflowStatusService } from
"../../service/workflow-status/workflow-status.service";
+import { OperatorPerformanceMetrics } from
"../../service/workflow-status/performance-metrics";
+import {
+ formatMetricForView,
+ HeatmapView,
+ heatmapViewTitle,
+ rawMetricForView,
+} from "../../service/heatmap/heatmap-scoring";
+import { scoreToColor } from "../../service/heatmap/heatmap-color";
+
+interface HeatmapLegendState {
+ readonly view: HeatmapView;
Review Comment:
`view` is assigned at `:78` and never read — the template uses only `title`,
`minLabel` and `maxLabel`. Worth dropping both lines.
##########
frontend/src/app/workspace/service/heatmap/heatmap-scoring.ts:
##########
@@ -0,0 +1,159 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { OperatorPerformanceMetrics } from
"../workflow-status/performance-metrics";
+
+/**
+ * The three heat-map views. Each answers a different "where should I look?"
+ * question; see {@link rawMetricForView} for the per-operator cost each uses.
+ * String-valued so the selection serializes readably (e.g. to localStorage).
+ */
+export enum HeatmapView {
+ Runtime = "runtime",
+ // Seconds per output row — the reciprocal of throughput, so it grows
(hotter) as throughput
+ // falls. Named directionally rather than "throughput" to match how the ramp
reads.
+ TimePerRow = "time-per-row",
+ IoImbalance = "io-imbalance",
+}
+
+/**
+ * Per-operator raw cost for a view, BEFORE normalization. Higher = hotter
(more
+ * worth a look).
+ *
+ * - Runtime: data + control processing time — slower operators are hotter.
+ * - TimePerRow: seconds per output tuple — slow producers (low throughput)
are
+ * hotter; no output -> 0 (cold).
+ * - IoImbalance: |out - in| / (out + in) — operators that drop OR amplify
rows are
+ * hotter; a balanced operator or missing input -> 0 (cold).
Normalized
+ * to [0, 1] so an extreme amplifier can't dominate the scale.
+ */
+export function rawMetricForView(metrics: OperatorPerformanceMetrics, view:
HeatmapView): number {
+ switch (view) {
+ case HeatmapView.Runtime:
+ return metrics.dataProcessingTimeNs + metrics.controlProcessingTimeNs;
+ case HeatmapView.TimePerRow: {
+ const timeSec = (metrics.dataProcessingTimeNs +
metrics.controlProcessingTimeNs) / 1e9;
+ return metrics.outputRows > 0 ? timeSec / metrics.outputRows : 0;
Review Comment:
`0` means "not measurable for this view" here, but it is also what a
genuinely fast operator produces. `normalizeScores` maps the minimum to score
0, so a blocking operator mid-run that has consumed rows and emitted none
paints the coldest thing on the canvas. It anchors the scale minimum and the
legend's low label too. `:55` collides the same way for a source, where `0` is
also the value for a balanced operator.
The overlay already encodes "no signal": an `undefined` score paints
`HEATMAP_NO_DATA_COLOR`. Dropping these operators from `rawById` in
`heatmapScores` (`workflow-editor.component.ts:482`) routes them there.
`heatmap-scoring.spec.ts:56` and `:95` pin the current behaviour.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
