1921175 - Unused methods exist around value grips

Status: RESOLVED FIXED

(DevTools :: Debugger, task)

Comment 1

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Remove unused createEnvironmentActor method from ObjectActor hooks.

Comment 2

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Remove unused ObjectActor.threadGrip method.

This was only used in test and was involving the promote hooks method.

Comment 3

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Remove unused pauseGrip and getProperties methods.

I kept the method on the front as many xpcshell test rely on this to simply
instantiate a front and it is tedious to create an object front via DevToolsClient.createObjectFront.

Comment 4

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Use ThreadActor helper to create grip from the environment actor.

(getParent() was equal to the thread actor)

Comment 5

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Create object grip from a unique method in the thread actor.

It sounds safer and easier to always check if the thread is paused
and _pausePool is defined to register the object actor in the right pool.

Comment 6

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

try is green:
https://treeherder.mozilla.org/jobs?repo=try&revision=4bcb381be1f818e0924a62a8d0e2750df813b729

Comment 7

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Revise ObjectActor arguments.

The actor objects usualy receive a connection as first argument
and thread attribute isn't used in hooks.

Comment 8

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Rename ObjectActor.thread to threadActor.

Comment 9

[Alexandre Poirot [:ochameau] [PTO back Jan 5th]]

Attachment: Bug 1921175 - [devtools] Fix PausedScopeObjectActor.isThreadLifetimePool.

It's condition was the opposite of its meaning...
and it is only used within the class, so no need to expose it on hooks.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/985b7383feaa
[devtools] Remove unused createEnvironmentActor method from ObjectActor hooks. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/a95a3f0ebccd
[devtools] Remove unused ObjectActor.threadGrip method. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/0a821ddf28a1
[devtools] Remove unused pauseGrip and getProperties methods. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/d8faa541e2ad
[devtools] Use ThreadActor helper to create grip from the environment actor. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/70bf92d8c197
[devtools] Create object grip from a unique method in the thread actor. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fe5f26df8632
[devtools] Revise ObjectActor arguments. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/b52f4af8feeb
[devtools] Rename ObjectActor.thread to threadActor. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/bfa6b93cf1c5
[devtools] Fix PausedScopeObjectActor.isThreadLifetimePool. r=devtools-reviewers,nchevobbe

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/985b7383feaa
https://hg.mozilla.org/mozilla-central/rev/a95a3f0ebccd
https://hg.mozilla.org/mozilla-central/rev/0a821ddf28a1
https://hg.mozilla.org/mozilla-central/rev/d8faa541e2ad
https://hg.mozilla.org/mozilla-central/rev/70bf92d8c197
https://hg.mozilla.org/mozilla-central/rev/fe5f26df8632
https://hg.mozilla.org/mozilla-central/rev/b52f4af8feeb
https://hg.mozilla.org/mozilla-central/rev/bfa6b93cf1c5

Comment 12

[Nicolas Chevobbe [:nchevobbe][PTO - back January 2nd]]

Perfherder has detected a devtools performance change from push 6d1113bb77a8bd300a5136c427d49bd55c64902f.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
7%	damp console.log-in-loop-content-process-document	windows11-64-shippable-qr	e10s fission stylo webrender	83.85 -> 77.61
6%	damp console.log-in-loop-content-process-document	linux1804-64-shippable-qr	e10s fission stylo webrender	270.49 -> 253.88
5%	damp console.log-in-loop-content-process-document	macosx1015-64-shippable-qr	e10s fission stylo webrender	225.54 -> 214.32
3%	damp console.log-in-loop-content-process-window	windows11-64-shippable-qr	e10s fission stylo webrender	248.68 -> 240.38

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 2329

For more information on performance sheriffing please see our FAQ.