Update the TracingMarkers to be used in the MarkerTable component

[gregtatum]

This PR updates both the Redux store and the MarkerTable component to use TracingMarkers. It takes us towards the path of consolidating how we use markers, and is based off of the findings of my markers work from this past week.

Rather than switching between the MarkersTable and TracingMarkers, eventually we can have one data structure that has all the processing we need done to it. I was originally leaning towards doing this step in profile processing, but keeping it dynamic will avoid the profile upgrading process, and makes it easier to split this work up into smaller parts. TracingMarkers are probably not the final shape of what this structure could look like, but it's a smaller footprint to moving to a more unified marker view that is easier to work with in a consistent manner.

This change allows for a one-step processing of TracingMakers in the Redux store that is easily memoizable. This processing takes the form:

• getProcessedMarkersTable - Process marker payloads out of raw strings, and other future processing needs.
• getTracingMarkers - Match up start/end markers
• getCommittedRangeFilteredTracingMarkers - Match up start/end markers, and remove duplicates.
• getSearchFilteredTracingMarkers - Apply the search string
• getPreviewFilteredTracingMarkers - Apply the preview range

This will work will make these follow-ups easier:

• Search filtering the MarkerChart, that is consistent with the MarkerChart.
• Tracing Markers can be the common processed format for various views like the Screenshots, and the Network.
• Future refactors can focus on making the TracingMarker information more precise.
• Future refactors can make it clearer that the MarkersTable is not a processed format, and give a clear space to do that work.

In addition, I added a bit of polish to the Marker Table, since we have more information about what is going on with it.

Deploy preview with marker table

Incidentally resolves #1195

[codecov]

Codecov Report

Merging #1227 into master will increase coverage by 0.02%.
The diff coverage is 68.75%.

@@            Coverage Diff             @@
##           master    #1227      +/-   ##
==========================================
+ Coverage   76.45%   76.48%   +0.02%     
==========================================
  Files         147      147              
  Lines        9621     9632      +11     
  Branches     2366     2373       +7     
==========================================
+ Hits         7356     7367      +11     
  Misses       2024     2024              
  Partials      241      241

Impacted Files	Coverage Δ
src/components/marker-table/ContextMenu.js	12.5% <0%> (ø)	⬆️
src/profile-logic/marker-data.js	78.98% <42.85%> (+4.15%)	⬆️
src/components/marker-table/index.js	73.07% <74.19%> (-3.49%)	⬇️
src/reducers/profile-view.js	94.44% <88.23%> (-0.37%)	⬇️

---

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 90b19f9...2d86bb8. Read the comment docs.

[julienw]

This is mostly OK, except for the markers where parse some payload out of their names: the fact you changed the logic to keep the previous name makes the marker chart messy, and I think we need a better solution. I agree that replacing the name by simpler names is some direct data loss, and we should be able to keep both, because we need both.

Another concern I have is the memory usage. It won't change with this patch (after all, we already had the tracing markers before), but we may want to convert the array of tracing markers to a flatter table like where we have our other data. But this is definitely not for this PR because this is a preexisting problem.

"request changes" because I want that we sort out the first issue.

[gregtatum]

@julienw this is ready for re-review.

[gregtatum]

Another concern I have is the memory usage. It won't change with this patch (after all, we already had the tracing markers before), but we may want to convert the array of tracing markers to a flatter table like where we have our other data. But this is definitely not for this PR because this is a preexisting problem.

Yes we should re-evaluate it. I started doing this in my monstrous markers refactor which shall not land. I am curious to measure the impact and see if it's actually slow. We end up retaining the tracing markers once they are created, so we don't have intermediate versions as of right now that would put pressure on the garbage collector.

[julienw]

We end up retaining the tracing markers once they are created, so we don't have intermediate versions as of right now that would put pressure on the garbage collector.

Yep that's right -- as always, we must first measure, and not fix what's not broken :)

[julienw]

Thanks !
let's land this :)