Page MenuHomePhabricator

Reduce flamegraph.pl threshold from minwidth=2 to minwidth=1
Closed, ResolvedPublic

Assigned To
Authored By
Krinkle
Mar 15 2020, 11:34 PM
Referenced Files
F31836115: before-1.png
May 21 2020, 7:08 PM
F31836119: after1.png
May 21 2020, 7:08 PM
F31836117: before-2.png
May 21 2020, 7:08 PM
F31683241: Screenshot 2020-03-15 at 23.32.40.png
Mar 15 2020, 11:34 PM
F31683237: Screenshot 2020-03-15 at 23.30.35.png
Mar 15 2020, 11:34 PM
F31683238: Screenshot 2020-03-15 at 23.30.52.png
Mar 15 2020, 11:34 PM

Description

As more indirection gets introduced in MediaWiki via factories and service classes, the stack traces are gaining two or three extra layers around the bottom of the chart, and also 2-3 layers toward the peak of any flame.

The peaks are where most of the leaf nodes are, and is where most of the "real" computations happen. Omitting narrow and tall sub stacks from that is fine if they are insignificant in terms of time spent overall. But, the threshold is based the pixel-width at the highest zoom level (zoomed out).

I'm noticing that more and more often the interesting bits in the peak are now cut off, and thus I can no longer see where peaks are spending there time (and thus not know what could be optimized in them).

For example, getKnownCurrentRevision does not have any children in this graph:

Screenshot 2020-03-15 at 23.30.35.png (1×2 px, 157 KB)

Screenshot 2020-03-15 at 23.30.52.png (1×2 px, 349 KB)

In this case I was able to use the reverse graph to find a few more frames, but that's not great from a UX perspective, and won't always work:

Screenshot 2020-03-15 at 23.32.40.png (1×2 px, 138 KB)

We added this originally in 2017 (patch), to reduce the size of the SVG files and to keep them fast to render.

The FlameGraph upstream default for minwidth is 0.1px. Perhaps we can try using --minwidth=1 and see how large our files would be, and if they still render at an acceptable speed.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

As this would increase storage footprint, we'll likely have to shelve this until we've migrated Arc Lamp's storage from local VM disk to Swift.

See: T227026: Deploy ArcLamp process as stateless/scalable service (Kubernetes)

Krinkle triaged this task as Medium priority.Mar 16 2020, 9:37 PM
Krinkle moved this task from Inbox, needs triage to Doing (old) on the Performance-Team board.

Per @aaron, during investigation of T247611 he also noticed important bits being cut off in the flame graph.

Change 597839 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[operations/puppet@production] arclamp: set --minwidth to 1 for all SVG flame graphs

https://gerrit.wikimedia.org/r/597839

Below a before-after of the upcoming patch for dedicated SubmitAction graphs:

Before
before-1.png (1×2 px, 582 KB)
(highlighted: the part we're intersted in)
before-2.png (1×2 px, 148 KB)
(zoom-in on what we're interested in)
After
after1.png (1×2 px, 747 KB)

Change 597839 merged by RLazarus:
[operations/puppet@production] arclamp: set --minwidth to 1 for all SVG flame graphs

https://gerrit.wikimedia.org/r/597839