795969 - Promote list of pause actors at once

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Upgrading actors will be often done for several actors. For example, expanding an actor in the DOM panel should show list of children (objects) actors that should all be promoted to thread actors (since the DOM panel deals with thread scoped actors).

So, it would be useful to have a new packet type that allows to promote list of given actors.

Honza

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v1

This should do it. I also made protocol logging in the local transport case pretty again per Dave's suggestion in London. The packet format mirrors the releaseMany packet:
{
  "to": "conn0.context2",
  "type": "threadGrips",
  "actors": [
    "conn0.obj14",
    "conn0.obj15",
    "conn0.obj16"
  ]
}
{
  "from": "conn0.context2"
}

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v2

I made another bug fix or optimization, depending on how you look at it: when creating object actors in a pause, the thread-lifetime pool is now consulted first, instead of just creating duplicates in the pause-lifetime pool. Moar tests.

Comment 3

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 670336 [details] [diff] [review]
Patch v2

missed this earlier.

objectGrip: function TA_objectGrip(aValue, aPool) {
     if (!aPool.objectActors) {
       aPool.objectActors = new WeakMap();
     }

lazy initializing aPool's objectActors property.

Wonder if we can reorder this method to shortcut the check for aPool.objectActors.has(aValue) in the case when we do this? Not critical, just seems like a tiny optimization.

     if (aPool.objectActors.has(aValue)) {
       return aPool.objectActors.get(aValue).grip();
+    } else if (this.threadLifetimePool.objectActors.has(aValue)) {
+      return this.threadLifetimePool.objectActors.get(aValue).grip();
     }

are threadLifetimePool's objectActor's a lower priority than aPool's? Do these need to be coalesced somehow or are they suitably separate?

looks fine.

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> Comment on attachment 670336 [details] [diff] [review]
> Patch v2
> 
> missed this earlier.
> 
> objectGrip: function TA_objectGrip(aValue, aPool) {
>      if (!aPool.objectActors) {
>        aPool.objectActors = new WeakMap();
>      }
> 
> lazy initializing aPool's objectActors property.
> 
> Wonder if we can reorder this method to shortcut the check for
> aPool.objectActors.has(aValue) in the case when we do this? Not critical,
> just seems like a tiny optimization.

I'm going to remove both of these checks in bug 805769.

>      if (aPool.objectActors.has(aValue)) {
>        return aPool.objectActors.get(aValue).grip();
> +    } else if (this.threadLifetimePool.objectActors.has(aValue)) {
> +      return this.threadLifetimePool.objectActors.get(aValue).grip();
>      }
> 
> are threadLifetimePool's objectActor's a lower priority than aPool's? Do
> these need to be coalesced somehow or are they suitably separate?
> 
> looks fine.

Currently an object can be in either a thread- or a pause-lifetime pool and its assorted weakmap, so order doesn't really matter. I'm working on unifying the weakmaps in bug 805769.

Comment 5

[Jim Blandy :jimb]

Comment on attachment 670336 [details] [diff] [review]
Patch v2

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

Looks great!

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Fixed the harmless but confusing recursion in the threadLifetimePool getter and landed:

https://hg.mozilla.org/integration/fx-team/rev/e902df61b34e

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/mozilla-central/rev/e902df61b34e