1051106 - |wantScanResults| ends up with hundreds of copies of |firstScan|

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Andrew McCreight [:mccr8]]

The long running test logs from bug 1051051 show there are 505 copies of the function firstScan.  504 of these copies are in the wantScanResults in the WifiManager.

It seems bad that this array is getting so large.

It is also odd that all of the functions are identical (they have the exact same type, shape, atom, script and fun_callscope) but somehow we end up allocating a new object for each of them. I'm not sure if that's something that can be avoided.

Comment 1

[Andrew McCreight [:mccr8]]

As I said, these function objects are all identical, so they are probably not responsible for more than 20KB or so of leaks as far as I can tell, so this shouldn't really block on the basis of the quantity of leak.  I could be wrong.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

1986       self.waitForScan(function firstScan() {});

That'll create a new function object every time it's executed.

Comment 3

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

[Blocking Requested - why for this release]:

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We should take a patch if we get it but the amount of memory we're leaking here is going to be measured in KB so I wouldn't hold the release for it.

Comment 5

[Andrew McCreight [:mccr8]]

This is so little memory I'm just going to clear the dependence.

Comment 6

[bhavana bajaj [:bajaj]]

clearing the nom based on comment #5

Comment 7

[Nicholas Nethercote [inactive]]

> 1986       self.waitForScan(function firstScan() {});
> 
> That'll create a new function object every time it's executed.

And it's useless. It just adds a no-op function to an array, and later on that no-op function might be executed but since it's a no-op that doesn't achieve anything. It should be possible to simply remove that line. That will avoid creating the function objects and also avoid growing the array.

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: Remove a useless waitForScan() call

I tested this by checking that wi-fi still works on my Buri. It does. Not sure
what testing to do beyond that.

Comment 9

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8475658 [details] [diff] [review]
Remove a useless waitForScan() call

Review of attachment 8475658 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch, thank you. 
Maybe we should also clear the wantScanResults when we disable wifi in one of these two places 
1. http://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js?from=wifiworker.js#2637
2. http://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js?from=wifiworker.js#1971

Comment 10

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1435529802b8

Comment 11

[Nicholas Nethercote [inactive]]

> Maybe we should also clear the wantScanResults when we disable wifi in one
> of these two places 
> 1. http://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js?from=wifiworker.js#2637
> 2. http://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js?from=wifiworker.js#1971

I didn't do that, because that's a more involved change that I'm not comfortable with, and because it's not necessary to fix the reported problem. Please feel free to do it as a follow-up, though :)

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1435529802b8