Virtualized traced results list

[rubenvp8510]

Which problem is this PR solving?

• This PR is part of the solution of the Issue Performance for large number of traces #247, I still think we need to implement a new endpoint, but this definitely makes the UI more responsive. This is still work in progress.

Short description of the changes

• When the trace search page has too many traces to show, it makes the UI less responsive, almost unusable in some cases. This is because the amount of DOM objects that the browser needs to handle. With virtualization we only render and maintain objects that are visible to the user.

[objectiser]

@rubenvp8510 @tiffon Realise this is marked WIP, but just wanted to check if there is any chance this could make it into 1.11, or would it be best to leave for a patch release?

[rubenvp8510]

@objectiser This is almost ready, I need to make sure that one of the test pass, then I can remove the WIP. I think I can have this by tomorrow end of the day, but I need @tiffon feedback.

[pavolloffay]

That would be great, if you also could cut 1.1.1 release of UI it would be awesome.

[tiffon]

Awesome! Great thinking to virtualize the search results!

I ran into an issue with the layout where result items weren't positioned correctly:

(Note: I made the text transparent.)

Taking a step back, we're using ListView to add virtualized scrolling to the TraceTimelineViewer. Seems to make sense to stick with ListView to add virtualized scrolling to the search results, so we don't introduce a second solution for the same functionality in the application code. (We're currently using react-virtualized-select, so react-virtualized is already an indirect dependency, but we're not using it in any application code.)

What do you think?

[tiffon]

Also, seems like a test is broken.

[tiffon]

Looks great! A couple of small comments.

[tiffon]

For this, the <Tag...> seems pretty heavy. What about using something home-grown?

<li
  key={name}
  className="ResultItem--serviceTag"
  style={{ borderLeftColor: colorGenerator.getColorByKey(name) }}
>
  {name} <span className="ResultItem--serviceCount">{count}</span>
</li>

.ResultItem--serviceTag {
  background: #fff;
  border-left: 15px solid transparent;
  border-radius: 4px;
  box-shadow: 0 0 1px rgba(0, 0, 0, 0.5);
  color: rgba(0, 0, 0, 0.65);
  display: inline-block;
  font-size: 0.9em;
  line-height: 1;
  margin: 0.3em 0.35em;
  padding: 0.3em;
}

.ResultItem--serviceCount {
  font-size: 0.9em;
  opacity: 0.5;
  padding-left: 0.2em;
}

[tiffon]

I think this needs to have the initialDraw prop set, otherwise it will render 300 items, initially.

[tiffon]

Currently, this state is not being updated when new props come in, so selecting result items for comparison is not working.

Seems like using memoize-one, instead, would be less work than maintaining the state.

[codecov]

Codecov Report

Merging #337 into master will increase coverage by 0.03%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
+ Coverage   90.69%   90.73%   +0.03%     
==========================================
  Files         152      152              
  Lines        3290     3301      +11     
  Branches      666      667       +1     
==========================================
+ Hits         2984     2995      +11     
  Misses        251      251              
  Partials       55       55

Impacted Files	Coverage Δ
...cePage/TraceTimelineViewer/VirtualizedTraceView.js	89.09% <ø> (ø)	⬆️
...ger-ui/src/components/common/ListView/Positions.js	98.5% <ø> (ø)
...nts/SearchTracePage/SearchResults/DiffSelection.js	100% <ø> (ø)	⬆️
...onents/SearchTracePage/SearchResults/ResultItem.js	100% <100%> (ø)	⬆️
.../jaeger-ui/src/components/common/ListView/index.js	96.96% <100%> (ø)
.../components/SearchTracePage/SearchResults/index.js	80% <91.66%> (+5%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f78dcf...fb9aaf9. Read the comment docs.

[tiffon]

Overall, this looks great. But, in using it, it seems pretty choppy, not nearly as smooth as simply rendering all of the result items.

To explore where we might be able to gain some ground, I haphazardly made the following changes:

In ListView/index

• Calling .forceUpdate() through window.requestAnimationFrame(..), only

In ResultItem

• Replacing all Link, Row, Col, Tag with native HTML elements

For the search results, in the React annotations of the timings section in the Chrome dev tools profiler, the update cycle is comprised of:

• Reconciliation 1
• Update
• Reconciliation 2

Totals durations for a single update are:

• Current: ~322ms
• With changes: ~167ms

In the current version, Reconciliation 2 happens synchronously after the update. With the window.requestAnimationFrame(..) change, there is a gap between them, during which the UI can still scroll (is not frozen), and it feels a lot smoother.

I tested it out with a result set size of around 400 traces which, in total, had ~32k spans.