Bug 1703545 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

The test at runs the following test script:
```javascript
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.
The test at https://searchfox.org/mozilla-central/source/devtools/client/framework/test/allocations/browser_allocations_target.js runs the following test script:
```javascript
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.

Back to Bug 1703545 Comment 0