Skip to content

Deploy Tracks and other polish work#1185

Merged
gregtatum merged 54 commits into
productionfrom
master
Aug 8, 2018
Merged

Deploy Tracks and other polish work#1185
gregtatum merged 54 commits into
productionfrom
master

Conversation

@gregtatum

Copy link
Copy Markdown
Member
  • More GC markers
  • Sidebar polish
  • Better payloads for Frame Construction
  • Timeline tracks, and grouping the timeline by process
  • Remove chart left margin

PaulBone and others added 30 commits July 23, 2018 21:44
Change the code style for the GC optional marker fields from:

    field === undefined ? null : markerDetail(...)

To

    field ? markerDetail(...) : null

I'm about to add some details that will test for two or more fields, and
this will be a little bit neater.
Add the total number of tenured cells and tenured cells per second for GC
minor markers.

This patch also adds formatSI for formatting numbers with some SI units.
Add the numbers of cells allocated in the nursery vs the tenured heap for
between each GCMinor event.

Use this to also show the cells tenured statistic as a percentage.
* adjust title margins
* adjust alignments by using the same grid layout for the whole sidebar
* adjust max-heights for the sidebar's header
* adjust the (non-)wrapping behavior for titles
* make it easy to select the titles

Fixes #1117
This PR is for Bug 1414345.
We added a "Frame Construction" tracing marker there, and this is for making it look better.
Instead of calling CallTree.preloadChildrenCache, which allocates new
arrays for every parent in the call tree, implement a modified version
which stores everything needed in a couple of big arrays. Since the
new version is very specific to the flame graph, it is moved into
flame-graph.js instead, and CallTree.preloadChildrenCache is removed.
formatNumber now supports choosing the number of sagnificant digits to use.
Make use of this in formatSI and formatBytes.  Change from ones to thousands
after 10,000, but for the other numbers do this at 1,000,000 etc and explain
this choice with a comment.
The upgrading tests were hard to find, and inconsistent with the current
style of jest testing. This PR collects the upgrades into their own
file, and adds the more typical describe/it blocks that make the tests
run and fail in a more granular fashion.
gregtatum and others added 24 commits August 2, 2018 12:45
This merge includes work from my different PRs. They were all reviewed separately, but
merged in a single PR. This change was too large to do in situ, so the changes were
accumulated, the commits were reviewed in separate non-merged PRs.

See the following PRs for the reviews:

1. Timeline tracks store (plus this actual merge) #1115 
2. Timeline tracks UI 1152
3. Add profile upgrading to ensure pids on all threads #1163 
4. URL upgrades #1164
5. Timeline track component tests #1168
Fix the sidebar button margin to not break the tab bar
This PR adds "committed ranges" and "preview ranges".
getRangeSelectionFilteredThread is now called getPreviewFilteredThread
Improve flame graph performance by reducing heavy allocations (Merge PR #1170)
These patches add some new information to the GCMinor marker tooltips.

The corresponding Firefox change is: https://bugzilla.mozilla.org/show_bug.cgi?id=1473213 and we should wait for that to be reviewed before merging this PR in case there are any changes.
Fixes #619, fixes #1123.

Depending on the number of threads / tracks visible in the list at the
top, and the height of the header area, the list of tracks can be
vertically scrollable. The scrollbar for vertical scrolling causes a
bunch of problems, depending on the platform:
 - Dragging the scrollbar thumb is currently impossible because we
   override the mouse events to control the range selection.
 - On platforms with overlay scrollbars, the scrollbar overlaps the
   graphs and makes it hard to interact with the parts of the graphs
   close to the right edge.
 - On platforms with classic (non-overlay) scrollbars, the scrollbar
   subtracts available area from the contents and causes the time scale
   of the graphs to be misaligned with respect to the ruler above the
   scroll box. Furthermore, changing a track's visibility can flip cause
   the track list to flip between having a scrollbar and not having a
   scrollbar, which makes the graphs shift.

This commit addresses all these problems by reserving a fixed amount of
space for the scrollbar at all times. It sets an absolute width on both
the timeline ruler at the top *and* on the scrollable contents.
When there is a scrollbar, the scrollbar is displayed in the reserved
space, otherwise the space is empty.
The reserved space is always 15px wide, regardless of the actual width
of the scrollbar on the current platform. If the actual scrollbar is
narrower than the reserved space, there will be a small gap. If it is
wider than the reserved space, it will cover some of the contents but
but will not affect the contents' size.

We use calc() and viewport units (100vw) in order to compute an absolute
CSS length value that we can use for the graph width. This is needed so
that we can give the "insides" of the scrollable element a width that is
the same regardless of the actual width of the scrollbar on the current
platform. There does not seem to exist a way to use relative sizes
inside a scrollable element that "ignore" the scrollbar width; e.g.
"width: 100%" refers to the "inside width" of the parent element with
the scrollbar width already subtracted, and there's no pure-CSS way to
say "set this element's width to the *outer width* of its parent element".
@gregtatum gregtatum merged commit 2723417 into production Aug 8, 2018
@codecov

codecov Bot commented Aug 8, 2018

Copy link
Copy Markdown

Codecov Report

Merging #1185 into production will increase coverage by 2.43%.
The diff coverage is 86.45%.

Impacted file tree graph

@@              Coverage Diff               @@
##           production    #1185      +/-   ##
==============================================
+ Coverage        73.7%   76.13%   +2.43%     
==============================================
  Files             140      144       +4     
  Lines            8659     9417     +758     
  Branches         2038     2295     +257     
==============================================
+ Hits             6382     7170     +788     
+ Misses           2016     2008       -8     
+ Partials          261      239      -22
Impacted Files Coverage Δ
src/reducers/zipped-profiles.js 82.6% <ø> (ø) ⬆️
src/components/stack-chart/index.js 86.66% <ø> (ø) ⬆️
src/profile-logic/call-tree.js 97.2% <ø> (+0.92%) ⬆️
src/components/marker-chart/index.js 100% <ø> (ø) ⬆️
src/components/marker-table/index.js 68.75% <ø> (ø) ⬆️
src/components/app/Details.js 50% <ø> (ø) ⬆️
src/components/app/TabBar.js 70% <ø> (ø) ⬆️
src/components/calltree/CallTree.js 94.44% <ø> (ø) ⬆️
src/types/actions.js 0% <ø> (ø) ⬆️
src/components/flame-graph/FlameGraph.js 60% <ø> (ø) ⬆️
... and 45 more

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 cad3ea0...7010b01. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants