browser_allocations_target.js no longer asserts client destroy
Categories
(DevTools :: General, defect, P3)
Tracking
(firefox-esr78 unaffected, firefox87 unaffected, firefox88 unaffected, firefox89 fixed)
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox87 | --- | unaffected |
firefox88 | --- | unaffected |
firefox89 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
The test at https://searchfox.org/mozilla-central/source/devtools/client/framework/test/allocations/browser_allocations_target.js runs the following test script:
async function testScript(tab) {
const descriptor = await TabDescriptorFactory.createDescriptorForTab(tab);
const target = await descriptor.getTarget();
await target.attach();
// eslint-disable-next-line mozilla/no-arbitrary-setTimeout
await new Promise(resolve => setTimeout(resolve, 1000));
await target.destroy();
}
and measures how many objects are still tracked before and after GC.
However, after Bug 1631451, target.destroy()
no longer destroys the client, so we are not destroying as much as we are supposed to.
What is very surprising is that even though were are not closing the client, we are still destroying more objects than before. Before the patch, total-after-gc was around 2000, after the patch, it is around a 1000.
Logging calls to Pool::destroy confirms that we are destroying less Pools now.
The results are unclear to me. Is the test really asserting what it should? Are we actually holding onto 50% less objects, even though we no longer try to destroy the client?
If I add back a call to await descriptor.destroy()
(which will destroy the client) then we go back to 2000 objects. I feel like we should add it back in the test, the improvement was a false positive.
Assignee | ||
Comment 1•4 years ago
|
||
Updated•4 years ago
|
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Comment 3•4 years ago
|
||
Set release status flags based on info from the regressing bug 1631451
Assignee | ||
Updated•4 years ago
|
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6b3d4881c37b [devtools] Destroy descriptor in browser_allocations_target.js to keep asserting client destruction r=nchevobbe
Comment 5•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Description
•