1825339 - ObjectUtils.makeDebuggeeValue helper can be optimized

Status: RESOLVED FIXED

(DevTools :: Console, enhancement)

Description

[Alexandre Poirot [:ochameau]]

The following method is called once per console.log arguments (and may be from other places?) and be a source of slowdown.
This method becomes especially visible when using the JS Tracer, which logs lots of console messages.
It is unfortunate in the case of the JS Tracer as it only passes strings to console.log, which are Debuggee Values and do not need anything, we can return value as is...

https://searchfox.org/mozilla-central/rev/42747dfd314e4c939dc7c33a13e1a2fddf4926fc/devtools/server/actors/object/utils.js#427-442

function makeDebuggeeValue(targetActor, value) {
  if (isObject(value)) {
    try {
      const global = Cu.getGlobalForObject(value);
      const dbgGlobal = targetActor.dbg.makeGlobalObjectReference(global);
      return dbgGlobal.makeDebuggeeValue(value);
    } catch (ex) {
      // The above can throw an exception if value is not an actual object
      // or 'Object in compartment marked as invisible to Debugger'
    }
  }
  const dbgGlobal = targetActor.dbg.makeGlobalObjectReference(
    targetActor.window || targetActor.workerGlobal
  );
  return dbgGlobal.makeDebuggeeValue(value);
}

First, the isObject check is expensive as it force instantiating a cope of the object!
Whereas here we are trying to know if that's a primitive or not.

Then, for some reason targetActor.window getter appears to be slow. So it would be nice to avoid accessing it as much as possible.

Otherwise, for objects we have a weird logic where we assume we might throw,
but we most likely do not throw unless value is a primitive. So it sounds like we could get rid of the try/catch and only use the getGlobalForObject which should be safe for all the objects. At first sight, try and ours existing tests seem to confirm all objects can be logged correctly.

DAMP seems to report positive improvements on existing tests:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=056a283abaf9084d1349d002e9d3b909e20f6bee&originalSignature=4081483&newSignature=4081483&framework=12&application=&originalRevision=fadb1c5b484bfb5432d88cb635069c4019156240&page=1&showOnlyConfident=1
This only impact server side code, so that it is only visible on the log-something tests.

I've also witness unecessary overhead related to createValueForTarget, but this is significant harder to revise.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1825339 - [devtools] Speed up makeDebuggeeValue.

This method could early return for all primitive types as they already are "debuggee values".
Then, when we have non primitives, getGlobalForObject should work
and reliably return the value's global.
The global is needed as Debugger.Object's makeDebuggeeValue only works
for objects of the same global.

There is just this edgecase around object coming from globals which
are flagged as "invisible to debugger".
We might want to return a special debuggee value instead of trying
to instantiate this special Debugger.Object which mostly throws...

I've also updated and added meaningful comments to clarify all this code.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/201c404a4bce
[devtools] Speed up makeDebuggeeValue. r=devtools-reviewers,nchevobbe

Comment 3

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/201c404a4bce

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

== Change summary for alert #37924 (as of Fri, 31 Mar 2023 05:58:50 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
14%	damp console.log-in-loop-content-process-promise	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	56.70 -> 64.66
10%	damp console.log-in-loop-content-process-promise	windows10-64-shippable-qr	e10s fission stylo webrender	64.51 -> 70.71
7%	damp console.log-in-loop-content-process-promise	windows10-64-shippable-qr	e10s fission stylo webrender-sw	64.35 -> 69.13
4%	damp console.log-in-loop-content-process-symbol	windows10-64-shippable-qr	e10s fission stylo webrender-sw	55.94 -> 58.38
4%	damp console.log-in-loop-content-process-symbol	windows10-64-shippable-qr	e10s fission stylo webrender	56.01 -> 58.40
4%	damp console.log-in-loop-content-process-error	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	82.33 -> 85.76
4%	damp console.log-in-loop-content-process-document	windows10-64-shippable-qr	e10s fission stylo webrender	89.23 -> 92.83
4%	damp console.log-in-loop-content-process-document	windows10-64-shippable-qr	e10s fission stylo webrender-sw	89.22 -> 92.63
4%	damp console.log-in-loop-content-process-nodelist	windows10-64-shippable-qr	e10s fission stylo webrender	228.00 -> 236.34
3%	damp console.log-in-loop-content-process-nodelist	windows10-64-shippable-qr	e10s fission stylo webrender-sw	228.36 -> 236.17
...	...	...	...	...
3%	damp console.log-in-loop-content-process-date	windows10-64-shippable-qr	e10s fission stylo webrender	67.20 -> 68.98
3%	damp console.log-in-loop-content-process-string	windows10-64-shippable-qr	e10s fission stylo webrender-sw	70.17 -> 72.03
2%	damp console.log-in-loop-content-process-string	windows10-64-shippable-qr	e10s fission stylo webrender	69.87 -> 71.53
2%	damp console.log-in-loop-content-process-set	windows10-64-shippable-qr	e10s fission stylo webrender	203.33 -> 207.78
2%	damp console.log-in-loop-content-process-window	windows10-64-shippable-qr	e10s fission stylo webrender	208.98 -> 213.47

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
12%	damp console.log-in-loop-content-process-promise	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	64.71 -> 57.26
9%	damp console.log-in-loop-content-process-promise	windows10-64-shippable-qr	e10s fission stylo webrender	71.92 -> 65.50
8%	damp console.log-in-loop-content-process-symbol	windows10-64-shippable-qr	e10s fission stylo webrender-sw	59.00 -> 54.05
8%	damp console.log-in-loop-content-process-undefined	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	36.65 -> 33.70
8%	damp console.log-in-loop-content-process-promise	windows10-64-shippable-qr	e10s fission stylo webrender-sw	71.28 -> 65.92
...	...	...	...	...
2%	damp custom.webconsole.out-of-order	windows10-64-shippable-qr	e10s fission stylo webrender	1,589.50 -> 1,556.95

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=37924