Open Bug 1367445 Opened 5 years ago Updated 3 years ago

TabGroup::InBackground shouldn't allocate

Categories

(Core :: DOM: Content Processes, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: froydnj, Unassigned)

Details

Attachments

(1 file)

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+
Flags: needinfo?(nfroyd)
Priority: -- → P3

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)
Flags: needinfo?(nfroyd)
You need to log in before you can comment on or make changes to this bug.