Skip to content

Commit

Permalink
Bug 1875045 - [devtools] Release Object actors by bulk. r=devtools-re…
Browse files Browse the repository at this point in the history
…viewers,devtools-backward-compat-reviewers,nchevobbe

For now we were releasing object actors one by one.
This would force to send an individual RDP request for each of them.
The console often release all objects actors related to older console message
going over the maximum limit of displayed console messages (10k).
This can easily grow in a large number of actors to be released,
either if console message are receiving many arguments and/or
if many console are logged.

We have to have one request per target as the actors could only be reached
within same-thread actor.
In order to prepare for ObjectFront removal, introduce a target-scoped "Objects" actor
which is a singleton per Target. It will receive the new "release in bulk objects actors"
method. Later, it will start implementing all the existing methods of the Object Actor
in order to migrate away from having to instantiate one Object Front (notice the singular on "Object"),
per inspected JS Object.

On the fronted side a new Object Command is introduced in order to abstract away the RDP/Fronts work.

Differential Revision: https://phabricator.services.mozilla.com/D198784
  • Loading branch information
ochameau committed Jan 29, 2024
1 parent bd77930 commit 6b018ef
Show file tree
Hide file tree
Showing 24 changed files with 439 additions and 46 deletions.
1 change: 1 addition & 0 deletions devtools/client/fronts/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ DevToolsModules(
"network-parent.js",
"node.js",
"object.js",
"objects-manager.js",
"page-style.js",
"perf.js",
"preference.js",
Expand Down
25 changes: 25 additions & 0 deletions devtools/client/fronts/objects-manager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

"use strict";

const {
objectsManagerSpec,
} = require("resource://devtools/shared/specs/objects-manager.js");
const {
FrontClassWithSpec,
registerFront,
} = require("resource://devtools/shared/protocol.js");

class ObjectsManagerFront extends FrontClassWithSpec(objectsManagerSpec) {
constructor(client, targetFront, parentFront) {
super(client, targetFront, parentFront);

// Attribute name from which to retrieve the actorID out of the target actor's form
this.formAttributeName = "objectsManagerActor";
}
}

module.exports = ObjectsManagerFront;
registerFront(ObjectsManagerFront);
32 changes: 13 additions & 19 deletions devtools/client/webconsole/enhancers/actor-releaser.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,19 @@ function enableActorReleaser(webConsoleUI) {
webConsoleUI &&
[MESSAGES_ADD, MESSAGES_CLEAR, PRIVATE_MESSAGES_CLEAR].includes(type)
) {
const promises = [];
state.messages.frontsToRelease.forEach(front => {
// We only release the front if it actually has a release method, if it isn't
// already destroyed, and if it's not in the sidebar (where we might still need it).
if (
front &&
typeof front.release === "function" &&
!front.isDestroyed() &&
(!state.ui.frontInSidebar ||
state.ui.frontInSidebar.actorID !== front.actorID)
) {
promises.push(front.release());
}
});

// Emit an event we can listen to to make sure all the fronts were released.
Promise.all(promises).then(() =>
webConsoleUI.emitForTests("fronts-released")
);
const { frontInSidebar } = state.ui;
let { frontsToRelease } = state.messages;
// Ignore the front for object still displayed in the sidebar, if there is one.
frontsToRelease = frontInSidebar
? frontsToRelease.filter(
front => frontInSidebar.actorID !== front.actorID
)
: state.messages.frontsToRelease;

webConsoleUI.hud.commands.objectCommand
.releaseObjects(frontsToRelease)
// Emit an event we can listen to to make sure all the fronts were released.
.then(() => webConsoleUI.emitForTests("fronts-released"));

// Reset `frontsToRelease` in message reducer.
state = reducer(state, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ async function getWebConsoleWrapper() {
const hud = {
currentTarget: { client: {}, getFront: () => {} },
getMappedExpression: () => {},
commands: {
objectCommand: {
releaseObjects: async frontsToRelease => {},
},
},
};
const webConsoleUi = getWebConsoleUiMock(hud);

Expand Down
16 changes: 15 additions & 1 deletion devtools/client/webconsole/test/node/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,21 @@ function getWebConsoleUiMock(hud) {
return {
emit: () => {},
emitForTests: () => {},
hud,
hud: {
// @backward-compat { version 123 } A new Objects Manager front has a new "releaseActors" method.
// Once 123 is release, supportsReleaseActors could be removed.
commands: {
client: {
mainRoot: {
supportsReleaseActors: true,
},
},
objectCommand: {
releaseObjects: async frontsToRelease => {},
},
},
...hud,
},
clearNetworkRequests: () => {},
clearMessagesCache: () => {},
inspectObjectActor: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const {
getFirstMessage,
getLastMessage,
getPrivatePacket,
getWebConsoleUiMock,
setupActions,
setupStore,
} = require("resource://devtools/client/webconsole/test/node/helpers.js");
Expand Down Expand Up @@ -202,7 +203,24 @@ describe("private messages", () => {

it("releases private backend actors on PRIVATE_MESSAGES_CLEAR action", () => {
const releasedActors = [];
const { dispatch, getState } = setupStore([]);
const { dispatch, getState } = setupStore([], {
webConsoleUI: getWebConsoleUiMock({
commands: {
client: {
mainRoot: {
supportsReleaseActors: true,
},
},
objectCommand: {
releaseObjects: async frontsToRelease => {
for (const front of frontsToRelease) {
releasedActors.push(front.actorID);
}
},
},
},
}),
});
const mockFrontRelease = function () {
releasedActors.push(this.actorID);
};
Expand Down
77 changes: 54 additions & 23 deletions devtools/client/webconsole/test/node/store/release-actors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const {
getFirstMessage,
setupActions,
setupStore,
getWebConsoleUiMock,
} = require("resource://devtools/client/webconsole/test/node/helpers.js");

const {
Expand All @@ -24,35 +25,45 @@ describe("Release actor enhancer:", () => {
it("releases backend actors when limit reached adding a single message", () => {
const logLimit = 100;
const releasedActors = [];
const mockFrontRelease = function () {
releasedActors.push(this.actorID);
};

const { dispatch, getState } = setupStore([], {
storeOptions: { logLimit },
webConsoleUI: getWebConsoleUiMock({
commands: {
client: {
mainRoot: {
supportsReleaseActors: true,
},
},
objectCommand: {
releaseObjects: async frontsToRelease => {
for (const front of frontsToRelease) {
releasedActors.push(front.actorID);
}
},
},
},
}),
});

// Add a log message.
const packet = stubPackets.get(
"console.log('myarray', ['red', 'green', 'blue'])"
);
packet.message.arguments[1].release = mockFrontRelease;
dispatch(actions.messagesAdd([packet]));

const firstMessage = getFirstMessage(getState());
const firstMessageActor = firstMessage.parameters[1].actorID;

// Add an evaluation result message (see Bug 1408321).
const evaluationResultPacket = stubPackets.get("new Date(0)");
evaluationResultPacket.result.release = mockFrontRelease;
dispatch(actions.messagesAdd([evaluationResultPacket]));
const secondMessageActor = evaluationResultPacket.result.actorID;

const logCount = logLimit + 1;
const assertPacket = stubPackets.get(
"console.assert(false, {message: 'foobar'})"
);
assertPacket.message.arguments[0].release = mockFrontRelease;
const thirdMessageActor = assertPacket.message.arguments[0].actorID;

for (let i = 1; i <= logCount; i++) {
Expand All @@ -71,33 +82,42 @@ describe("Release actor enhancer:", () => {
const releasedActors = [];
const { dispatch, getState } = setupStore([], {
storeOptions: { logLimit },
webConsoleUI: getWebConsoleUiMock({
commands: {
client: {
mainRoot: {
supportsReleaseActors: true,
},
},
objectCommand: {
releaseObjects: async frontsToRelease => {
for (const front of frontsToRelease) {
releasedActors.push(front.actorID);
}
},
},
},
}),
});

const mockFrontRelease = function () {
releasedActors.push(this.actorID);
};

// Add a log message.
const logPacket = stubPackets.get(
"console.log('myarray', ['red', 'green', 'blue'])"
);
logPacket.message.arguments[1].release = mockFrontRelease;
dispatch(actions.messagesAdd([logPacket]));

const firstMessage = getFirstMessage(getState());
const firstMessageActor = firstMessage.parameters[1].actorID;

// Add an evaluation result message (see Bug 1408321).
const evaluationResultPacket = stubPackets.get("new Date(0)");
evaluationResultPacket.result.release = mockFrontRelease;
dispatch(actions.messagesAdd([evaluationResultPacket]));
const secondMessageActor = evaluationResultPacket.result.actorID;

// Add an assertion message.
const assertPacket = stubPackets.get(
"console.assert(false, {message: 'foobar'})"
);
assertPacket.message.arguments[0].release = mockFrontRelease;
dispatch(actions.messagesAdd([assertPacket]));
const thirdMessageActor = assertPacket.message.arguments[0].actorID;

Expand All @@ -122,17 +142,29 @@ describe("Release actor enhancer:", () => {

it("properly releases backend actors after clear", () => {
const releasedActors = [];
const { dispatch, getState } = setupStore([]);

const mockFrontRelease = function () {
releasedActors.push(this.actorID);
};
const { dispatch, getState } = setupStore([], {
webConsoleUI: getWebConsoleUiMock({
commands: {
client: {
mainRoot: {
supportsReleaseActors: true,
},
},
objectCommand: {
releaseObjects: async frontsToRelease => {
for (const front of frontsToRelease) {
releasedActors.push(front.actorID);
}
},
},
},
}),
});

// Add a log message.
const logPacket = stubPackets.get(
"console.log('myarray', ['red', 'green', 'blue'])"
);
logPacket.message.arguments[1].release = mockFrontRelease;
dispatch(actions.messagesAdd([logPacket]));

const firstMessage = getFirstMessage(getState());
Expand All @@ -142,31 +174,30 @@ describe("Release actor enhancer:", () => {
const assertPacket = stubPackets.get(
"console.assert(false, {message: 'foobar'})"
);
assertPacket.message.arguments[0].release = mockFrontRelease;
dispatch(actions.messagesAdd([assertPacket]));
const secondMessageActor = assertPacket.message.arguments[0].actorID;

// Add an evaluation result message (see Bug 1408321).
const evaluationResultPacket = stubPackets.get("new Date(0)");
evaluationResultPacket.result.release = mockFrontRelease;
dispatch(actions.messagesAdd([evaluationResultPacket]));
const thirdMessageActor = evaluationResultPacket.result.actorID;

// Add a message with a long string messageText property.
const longStringPacket = stubPackets.get("TypeError longString message");
longStringPacket.pageError.errorMessage.release = mockFrontRelease;
dispatch(actions.messagesAdd([longStringPacket]));
const fourthMessageActor =
longStringPacket.pageError.errorMessage.actorID;
const fifthMessageActor = longStringPacket.pageError.exception.actorID;

// Kick-off the actor release.
dispatch(actions.messagesClear());

expect(releasedActors.length).toBe(4);
expect(releasedActors.length).toBe(5);
expect(releasedActors).toInclude(firstMessageActor);
expect(releasedActors).toInclude(secondMessageActor);
expect(releasedActors).toInclude(thirdMessageActor);
expect(releasedActors).toInclude(fourthMessageActor);
expect(releasedActors).toInclude(fifthMessageActor);
});
});
});
1 change: 1 addition & 0 deletions devtools/server/actors/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ DevToolsModules(
"manifest.js",
"memory.js",
"object.js",
"objects-manager.js",
"page-style.js",
"pause-scoped.js",
"perf.js",
Expand Down
2 changes: 2 additions & 0 deletions devtools/server/actors/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,8 @@ class ObjectActor extends Actor {
this.hooks.customFormatterConfigDbgObj = null;
}
this._customFormatterItem = null;
this.obj = null;
this.thread = null;
}
}

Expand Down
39 changes: 39 additions & 0 deletions devtools/server/actors/objects-manager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

"use strict";

const { Actor } = require("resource://devtools/shared/protocol.js");
const {
objectsManagerSpec,
} = require("resource://devtools/shared/specs/objects-manager.js");

/**
* This actor is a singleton per Target which allows interacting with JS Object
* inspected by DevTools. Typically from the Console or Debugger.
*/
class ObjectsManagerActor extends Actor {
constructor(conn, targetActor) {
super(conn, objectsManagerSpec);
}

/**
* Release Actors by bulk by specifying their actor IDs.
* (Passing the whole Front [i.e. Actor's form] would be more expensive than passing only their IDs)
*
* @param {Array<string>} actorIDs
* List of all actor's IDs to release.
*/
releaseObjects(actorIDs) {
for (const actorID of actorIDs) {
const actor = this.conn.getActor(actorID);
// Note that release will also typically call Actor's destroy and unregister the actor from its Pool
if (actor) {
actor.release();
}
}
}
}

exports.ObjectsManagerActor = ObjectsManagerActor;
2 changes: 2 additions & 0 deletions devtools/server/actors/root.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ class RootActor extends Actor {
"dom.worker.console.dispatch_events_to_main_thread"
)
: true,
// @backward-compat { version 123 } A new Objects Manager front has a new "releaseActors" method.
supportsReleaseActors: true,
};
}

Expand Down
Loading

0 comments on commit 6b018ef

Please sign in to comment.