Open
Bug 1367445
Opened 5 years ago
Updated 3 years ago
TabGroup::InBackground shouldn't allocate
Categories
(Core :: DOM: Content Processes, enhancement, P3)
Core
DOM: Content Processes
Tracking
()
NEW
People
(Reporter: froydnj, Unassigned)
Details
Attachments
(1 file)
2.22 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
I saw this function in profiles taking up a decent slice of time; the nsGlobalWindow checks it has to do are kind of expensive, but we should at least be able to not allocate during this function. WDYT? Is this worth it?
Attachment #8870837 -
Flags: review?(wmccloskey)
Comment on attachment 8870837 [details] [diff] [review] make TabGroup::InBackground not allocate memory Review of attachment 8870837 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/TabGroup.cpp @@ +228,2 @@ > > for (nsPIDOMWindowOuter* outerWindow : mWindows) { I wonder if we could keep a separate array of top-level windows. I think we know whether a window is top-level or not when it Joins the TabGroup. ::: dom/base/TabGroup.h @@ +129,5 @@ > + // Iterate over each toplevel window, returning true from F to continue > + // iterating, and false to stop iterating. Involves no allocations, unlike > + // iterating over the result of GetTopLevelWindows(). > + template<typename F> > + void ForEachTopLevelWindow(F&& aFunc) const; I guess we can't make this public because we'd have to inline it? What if we kept the "is it a top-level window" test as a private member function, but inlined the loop?
Attachment #8870837 -
Flags: review?(wmccloskey) → review+
![]() |
||
Updated•5 years ago
|
Flags: needinfo?(nfroyd)
Priority: -- → P3
Comment 2•3 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:froydnj, could you have a look please?
Flags: needinfo?(nfroyd)
![]() |
||
Updated•3 years ago
|
Flags: needinfo?(nfroyd)
You need to log in
before you can comment on or make changes to this bug.
Description
•