From e6259db9ab32ebb9762dc063534673a15462b205 Mon Sep 17 00:00:00 2001
From: Ruben Vargas <ruben.vp8510@gmail.com>
Date: Wed, 26 Aug 2020 17:19:45 -0500
Subject: [PATCH 1/2] Fixes TraceTimelineViewer span details

Signed-off-by: Ruben Vargas <ruben.vp8510@gmail.com>
---
 .../VirtualizedTraceView.test.js              |  2 +-
 .../VirtualizedTraceView.tsx                  | 97 ++++++++++---------
 2 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
index d789c48b46..df79107432 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
@@ -305,7 +305,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
       expect(
         rowWrapper.containsMatchingElement(
           <SpanBarRow
-            className={instance.clippingCssClasses}
+            className={instance.getClippingCssClasses()}
             columnDivision={props.spanNameColumnWidth}
             isChildrenExpanded
             isDetailExpanded={false}
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index 4e4a8aa6a8..9b33ff50b4 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -20,6 +20,7 @@ import { withRouter, RouteComponentProps } from 'react-router-dom';
 
 // import { History as RouterHistory, Location } from 'history';
 
+import memoizeOne from 'memoize-one';
 import { actions } from './duck';
 import ListView from './ListView';
 import SpanBarRow from './SpanBarRow';
@@ -130,6 +131,14 @@ function generateRowStates(
   return rowStates;
 }
 
+function generateRowStatesFromTrace(
+  trace: Trace | TNil,
+  childrenHiddenIDs: Set<string>,
+  detailStates: Map<string, DetailState | TNil>
+): RowState[] {
+  return trace ? generateRowStates(trace.spans, childrenHiddenIDs, detailStates) : [];
+}
+
 function getCssClasses(currentViewRange: [number, number]) {
   const [zoomStart, zoomEnd] = currentViewRange;
   return cx({
@@ -138,28 +147,16 @@ function getCssClasses(currentViewRange: [number, number]) {
   });
 }
 
+const memoizedGenerateRowStates = memoizeOne(generateRowStatesFromTrace);
+const memoizedViewBoundsFunc = memoizeOne(createViewedBoundsFunc);
+const memoizedGetCssClasses = memoizeOne(getCssClasses);
+
 // export from tests
 export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceViewProps> {
-  clippingCssClasses: string;
   listView: ListView | TNil;
-  rowStates: RowState[];
-  getViewedBounds: ViewedBoundsFunctionType;
-
   constructor(props: VirtualizedTraceViewProps) {
     super(props);
-    // keep "prop derivations" on the instance instead of calculating in
-    // `.render()` to avoid recalculating in every invocation of `.renderRow()`
-    const { currentViewRangeTime, childrenHiddenIDs, detailStates, setTrace, trace, uiFind } = props;
-    this.clippingCssClasses = getCssClasses(currentViewRangeTime);
-    const [zoomStart, zoomEnd] = currentViewRangeTime;
-    this.getViewedBounds = createViewedBoundsFunc({
-      min: trace.startTime,
-      max: trace.endTime,
-      viewStart: zoomStart,
-      viewEnd: zoomEnd,
-    });
-    this.rowStates = generateRowStates(trace.spans, childrenHiddenIDs, detailStates);
-
+    const { setTrace, trace, uiFind } = props;
     setTrace(trace, uiFind);
   }
 
@@ -180,14 +177,11 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
   }
 
   componentDidUpdate(prevProps: Readonly<VirtualizedTraceViewProps>) {
-    const { childrenHiddenIDs, detailStates, registerAccessors, trace, currentViewRangeTime } = prevProps;
+    const { registerAccessors, trace } = prevProps;
     const {
       shouldScrollToFirstUiFindMatch,
       clearShouldScrollToFirstUiFindMatch,
       scrollToFirstVisibleSpan,
-      currentViewRangeTime: nextViewRangeTime,
-      childrenHiddenIDs: nextHiddenIDs,
-      detailStates: nextDetailStates,
       registerAccessors: nextRegisterAccessors,
       setTrace,
       trace: nextTrace,
@@ -198,21 +192,6 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
       setTrace(nextTrace, uiFind);
     }
 
-    if (trace !== nextTrace || childrenHiddenIDs !== nextHiddenIDs || detailStates !== nextDetailStates) {
-      this.rowStates = nextTrace ? generateRowStates(nextTrace.spans, nextHiddenIDs, nextDetailStates) : [];
-    }
-
-    if (currentViewRangeTime !== nextViewRangeTime) {
-      this.clippingCssClasses = getCssClasses(nextViewRangeTime);
-      const [zoomStart, zoomEnd] = nextViewRangeTime;
-      this.getViewedBounds = createViewedBoundsFunc({
-        min: trace.startTime,
-        max: trace.endTime,
-        viewStart: zoomStart,
-        viewEnd: zoomEnd,
-      });
-    }
-
     if (this.listView && registerAccessors !== nextRegisterAccessors) {
       nextRegisterAccessors(this.getAccessors());
     }
@@ -223,6 +202,28 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
     }
   }
 
+  getRowStates(): RowState[] {
+    const { childrenHiddenIDs, detailStates, trace } = this.props;
+    return memoizedGenerateRowStates(trace, childrenHiddenIDs, detailStates);
+  }
+
+  getClippingCssClasses(): string {
+    const { currentViewRangeTime } = this.props;
+    return memoizedGetCssClasses(currentViewRangeTime);
+  }
+
+  getViewedBounds(): ViewedBoundsFunctionType {
+    const { currentViewRangeTime, trace } = this.props;
+    const [zoomStart, zoomEnd] = currentViewRangeTime;
+
+    return memoizedViewBoundsFunc({
+      min: trace.startTime,
+      max: trace.endTime,
+      viewStart: zoomStart,
+      viewEnd: zoomEnd,
+    });
+  }
+
   focusSpan = (uiFind: string) => {
     const { trace, focusUiFindMatches, location, history } = this.props;
     if (trace) {
@@ -259,12 +260,12 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
 
   getCollapsedChildren = () => this.props.childrenHiddenIDs;
 
-  mapRowIndexToSpanIndex = (index: number) => this.rowStates[index].spanIndex;
+  mapRowIndexToSpanIndex = (index: number) => this.getRowStates()[index].spanIndex;
 
   mapSpanIndexToRowIndex = (index: number) => {
-    const max = this.rowStates.length;
+    const max = this.getRowStates().length;
     for (let i = 0; i < max; i++) {
-      const { spanIndex } = this.rowStates[i];
+      const { spanIndex } = this.getRowStates()[i];
       if (spanIndex === index) {
         return i;
       }
@@ -283,7 +284,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
   // use long form syntax to avert flow error
   // https://github.com/facebook/flow/issues/3076#issuecomment-290944051
   getKeyFromIndex = (index: number) => {
-    const { isDetail, span } = this.rowStates[index];
+    const { isDetail, span } = this.getRowStates()[index];
     return `${span.spanID}--${isDetail ? 'detail' : 'bar'}`;
   };
 
@@ -291,9 +292,9 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
     const parts = key.split('--');
     const _spanID = parts[0];
     const _isDetail = parts[1] === 'detail';
-    const max = this.rowStates.length;
+    const max = this.getRowStates().length;
     for (let i = 0; i < max; i++) {
-      const { span, isDetail } = this.rowStates[i];
+      const { span, isDetail } = this.getRowStates()[i];
       if (span.spanID === _spanID && isDetail === _isDetail) {
         return i;
       }
@@ -302,7 +303,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
   };
 
   getRowHeight = (index: number) => {
-    const { span, isDetail } = this.rowStates[index];
+    const { span, isDetail } = this.getRowStates()[index];
     if (!isDetail) {
       return DEFAULT_HEIGHTS.bar;
     }
@@ -315,7 +316,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
   linksGetter = (span: Span, items: KeyValuePair[], itemIndex: number) => getLinks(span, items, itemIndex);
 
   renderRow = (key: string, style: React.CSSProperties, index: number, attrs: {}) => {
-    const { isDetail, span, spanIndex } = this.rowStates[index];
+    const { isDetail, span, spanIndex } = this.getRowStates()[index];
     return isDetail
       ? this.renderSpanDetailRow(span, key, style, attrs)
       : this.renderSpanBarRow(span, spanIndex, key, style, attrs);
@@ -348,7 +349,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
     if (isCollapsed) {
       const rpcSpan = findServerChildSpan(trace.spans.slice(spanIndex));
       if (rpcSpan) {
-        const rpcViewBounds = this.getViewedBounds(rpcSpan.startTime, rpcSpan.startTime + rpcSpan.duration);
+        const rpcViewBounds = this.getViewedBounds()(rpcSpan.startTime, rpcSpan.startTime + rpcSpan.duration);
         rpc = {
           color: colorGenerator.getColorByKey(rpcSpan.process.serviceName),
           operationName: rpcSpan.operationName,
@@ -361,7 +362,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
     return (
       <div className="VirtualizedTraceView--row" key={key} style={style} {...attrs}>
         <SpanBarRow
-          className={this.clippingCssClasses}
+          className={this.getClippingCssClasses()}
           color={color}
           columnDivision={spanNameColumnWidth}
           isChildrenExpanded={!isCollapsed}
@@ -372,7 +373,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
           onChildrenToggled={childrenToggle}
           rpc={rpc}
           showErrorIcon={showErrorIcon}
-          getViewedBounds={this.getViewedBounds}
+          getViewedBounds={this.getViewedBounds()}
           traceStartTime={trace.startTime}
           span={span}
           focusSpan={this.focusSpan}
@@ -428,7 +429,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
       <div className="VirtualizedTraceView--spans">
         <ListView
           ref={this.setListView}
-          dataLength={this.rowStates.length}
+          dataLength={this.getRowStates().length}
           itemHeightGetter={this.getRowHeight}
           itemRenderer={this.renderRow}
           viewBuffer={300}

From ac10a5f5415d34a479c224d49744d7434791b1d7 Mon Sep 17 00:00:00 2001
From: Ruben Vargas <ruben.vp8510@gmail.com>
Date: Fri, 4 Sep 2020 14:23:15 -0500
Subject: [PATCH 2/2] perform deep comparision for memoized ViewBoundsFunc and
 GetCssClasses

Signed-off-by: Ruben Vargas <ruben.vp8510@gmail.com>
---
 .../TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx   | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
index 9b33ff50b4..8187a8a32b 100644
--- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
@@ -17,6 +17,7 @@ import cx from 'classnames';
 import { connect } from 'react-redux';
 import { bindActionCreators, Dispatch } from 'redux';
 import { withRouter, RouteComponentProps } from 'react-router-dom';
+import _isEqual from 'lodash/isEqual';
 
 // import { History as RouterHistory, Location } from 'history';
 
@@ -148,8 +149,8 @@ function getCssClasses(currentViewRange: [number, number]) {
 }
 
 const memoizedGenerateRowStates = memoizeOne(generateRowStatesFromTrace);
-const memoizedViewBoundsFunc = memoizeOne(createViewedBoundsFunc);
-const memoizedGetCssClasses = memoizeOne(getCssClasses);
+const memoizedViewBoundsFunc = memoizeOne(createViewedBoundsFunc, _isEqual);
+const memoizedGetCssClasses = memoizeOne(getCssClasses, _isEqual);
 
 // export from tests
 export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceViewProps> {