Closed Bug 1174965 Opened 9 years ago Closed 2 years ago

Share thread inflation amongst views

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P3)

defect

Tracking

(firefox41 affected)

RESOLVED WORKSFORME
Tracking Status
firefox41 --- affected

People

(Reporter: jsantell, Unassigned)

References

Details

Attachments

(1 file)

Right now, the call tree inflates the thread on every range or option change, and the flame graph inflates the entire thread on every option change. This loops over each sample and frame, inflating and doing view-specific things (creates a tree for calltree, determines positioning for flame graph). If we can consolidate the inflating for all these views and share the inflated thread, we can reduce reinflating logic (although individual inflated frames are cached), not have to reinflate on range change (more common than option change) on the call tree, possibly share data for an entire thread regardless of view (giving us possibly the ability to share computation of the entire thread in the flamegraph), and reuse inflating for a JIT view as well.
So right now, this maps a thread from the profiler data to an InflatedThread, that's computed the first time it's needed for the entire thread (no options filtering, no time filtering, etc). This means there'll be more overhead on first call tree usage as they don't share loops.

What I'm seeing is, yes the first time is a bit longer, but subsequent ThreadNode creations are still 30% or so slower than without this patch. Not sure how this can be the case, because subsequent ThreadNode ctors should be faster, or at the very least, equal. Must be some browser optimization I'm not understanding?

Haven't glued this into the FlameGraph, which does store the entire thread data upfront for smooth rendering. Can also be used to store info like total costs, etc, so that FlameGraph/JIT view can use those as well, rather than call tree just storing it.
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Attachment #8623123 - Flags: feedback?(shu)
Comment on attachment 8623123 [details] [diff] [review]
1174965-inflatedthread.patch

Review of attachment 8623123 [details] [diff] [review]:
-----------------------------------------------------------------

We talked about this on IRC.

To recap: currently the call tree model and the flame graph model are computing pretty different things -- a unified algorithm with equal performance to the current situation would require some more thought. I wouldn't land this patch. We'll talk more about this at Whistler.
Attachment #8623123 - Flags: feedback?(shu)
No longer blocks: 1150299
Not dealing with this for awhile.
Assignee: jsantell → nobody
Status: ASSIGNED → NEW
Triaging. Filter on ADRENOCORTICOTROPIC (yes).
Priority: -- → P3
Version: 41 Branch → unspecified
Product: Firefox → DevTools

The old panel is not used any more (bug 1693316), closing this bug.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: