Question about closure compilation
Hi, Does the closure returned from | foo | foo := #( data 1 2 3 ). [ :x | x + 1 ] close over foo ? Since foo is not used, it should not, right ? But it seems that the outer context of the block closure knows about foo, although the value seems nil. And if it closes over foo, foo's value/binding won't be GC-ed so long as there is a reference to the block closure, even though foo is not practically accessible. Sven
On Jun 28, 2013, at 9:26 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi,
Does the closure returned from
| foo | foo := #( data 1 2 3 ). [ :x | x + 1 ]
close over foo ?
No.
Since foo is not used, it should not, right ?
right. if we get the AST of your method ast := (TT>>#tt) ast. (ast scope lookupVar: 'foo') isEscaping --> false. Now the question is what the inspector of the context should show? e.g. tt | foo | foo := #( data 1 2 3 ). ^[ :x | x + 1. thisContext inspect ] if we now execute: TT new tt value: 2 the inspector pops up and shows both x and foo. The reason is that we need to decide what to show 1) the variable that is stored in the context 2) all *possible* variables that I can write in the block and recompile and it works. 1) is this: ast statements last value scope localTempNames 2) is this: ast statements last value scope allTemps Opal implements is like this because the old Compiler (DebuggerMethodMap) did it like this. At first I thought this is wrong and I wanted to change it. But then I thought that it is not that wrong: in the debugger the inspector of the context shows not just the vars that are in the context, but all that *could* be there. All where I can lookup and get a result, e.g. ast statements last value scope lookupVar: 'foo' Thus all those that I *could* write in the code inside the block. I am not sure, though⦠maybe we should change it. Marcus
But it seems that the outer context of the block closure knows about foo, although the value seems nil.
And if it closes over foo, foo's value/binding won't be GC-ed so long as there is a reference to the block closure, even though foo is not practically accessible.
Sven
On 28 Jun 2013, at 22:42, Marcus Denker <marcus.denker@inria.fr> wrote:
On Jun 28, 2013, at 9:26 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi,
Does the closure returned from
| foo | foo := #( data 1 2 3 ). [ :x | x + 1 ]
close over foo ?
No.
Since foo is not used, it should not, right ?
right. if we get the AST of your method
ast := (TT>>#tt) ast. (ast scope lookupVar: 'foo') isEscaping
--> false.
OK, that seems OK. The reason I was asking was because I wanted to know if this JavaScript bug would hit us as well: http://point.davidglasser.net/2013/06/27/surprising-javascript-memory-leak.h... It is slightly more complicated than the basic question of closing over an unreferenced variable though. I just can't figure out a way to quickly test it in Pharo. I think it is worth looking into.
Now the question is what the inspector of the context should show?
e.g.
tt | foo | foo := #( data 1 2 3 ). ^[ :x | x + 1. thisContext inspect ]
if we now execute:
TT new tt value: 2
the inspector pops up and shows both x and foo.
The reason is that we need to decide what to show
1) the variable that is stored in the context 2) all *possible* variables that I can write in the block and recompile and it works.
1) is this: ast statements last value scope localTempNames
2) is this:
ast statements last value scope allTemps
Opal implements is like this because the old Compiler (DebuggerMethodMap) did it like this. At first I thought this is wrong and I wanted to change it. But then I thought that it is not that wrong: in the debugger the inspector of the context shows not just the vars that are in the context, but all that *could* be there. All where I can lookup and get a result, e.g.
ast statements last value scope lookupVar: 'foo'
Thus all those that I *could* write in the code inside the block.
I am not sure, though⦠maybe we should change it.
Marcus
But it seems that the outer context of the block closure knows about foo, although the value seems nil.
And if it closes over foo, foo's value/binding won't be GC-ed so long as there is a reference to the block closure, even though foo is not practically accessible.
Sven
OK, that seems OK.
The reason I was asking was because I wanted to know if this JavaScript bug would hit us as well:
http://point.davidglasser.net/2013/06/27/surprising-javascript-memory-leak.h...
It is slightly more complicated than the basic question of closing over an unreferenced variable though.
I just can't figure out a way to quickly test it in Pharo. I think it is worth looking into.
Yes, we have exactly the same problem. imagine a method like this: test | bigObject smallObject | [bigObject := Array new: 1000000] value. ^[smallObject := 1]. both big and small are escaping variables that are written. This means they are allocated in an Array on the heap (Cog is calling this a TempVector). There is just one tempVector created per scope, so we will have a tempVector with both smallObject and bigObject inside created for the method. This then is passed to the blocks, which access the temps via the vector. The bytecode looks like this: 21 <8A 02> push: (Array new: 2) 23 <68> popIntoTemp: 0 24 <10> pushTemp: 0 25 <8F 10 00 07> closureNumCopied: 1 numArgs: 0 bytes 29 to 35 29 <40> pushLit: Array 30 <21> pushConstant: 1000000 31 <CD> send: new: 32 <8D 00 00> storeIntoTemp: 0 inVectorAt: 0 35 <7D> blockReturn 36 <C9> send: value 37 <87> pop 38 <10> pushTemp: 0 39 <8F 10 00 05> closureNumCopied: 1 numArgs: 0 bytes 43 to 47 43 <76> pushConstant: 1 44 <8D 01 00> storeIntoTemp: 1 inVectorAt: 0 47 <7D> blockReturn 48 <7C> returnTop The closure [smallObject := 1] now, even though not referencing the largeObject, references the tempVector. This tempVector holds on the to large object --> memory is wasted that is not needed. Back when implementing the SemChecker, I was thinking about this and wondering⦠for sure this can be optimized by allocating multiple arrays per scope, depending on how they are used later. But even then I am sure you can construct strange cases (multiple nested blocks). And of course allocating 2 Arrays (or more) wastes memory, too. So this is not a simple problem⦠the question is how much it matters in practice, though. Marcus
Hi Marcus, On 29 Jun 2013, at 11:12, Marcus Denker <marcus.denker@inria.fr> wrote:
OK, that seems OK.
The reason I was asking was because I wanted to know if this JavaScript bug would hit us as well:
http://point.davidglasser.net/2013/06/27/surprising-javascript-memory-leak.h...
It is slightly more complicated than the basic question of closing over an unreferenced variable though.
I just can't figure out a way to quickly test it in Pharo. I think it is worth looking into.
Yes, we have exactly the same problem.
imagine a method like this:
test | bigObject smallObject |
[bigObject := Array new: 1000000] value. ^[smallObject := 1].
both big and small are escaping variables that are written. This means they are allocated in an Array on the heap (Cog is calling this a TempVector).
There is just one tempVector created per scope, so we will have a tempVector with both smallObject and bigObject inside created for the method. This then is passed to the blocks, which access the temps via the vector.
The bytecode looks like this:
21 <8A 02> push: (Array new: 2) 23 <68> popIntoTemp: 0 24 <10> pushTemp: 0 25 <8F 10 00 07> closureNumCopied: 1 numArgs: 0 bytes 29 to 35 29 <40> pushLit: Array 30 <21> pushConstant: 1000000 31 <CD> send: new: 32 <8D 00 00> storeIntoTemp: 0 inVectorAt: 0 35 <7D> blockReturn 36 <C9> send: value 37 <87> pop 38 <10> pushTemp: 0 39 <8F 10 00 05> closureNumCopied: 1 numArgs: 0 bytes 43 to 47 43 <76> pushConstant: 1 44 <8D 01 00> storeIntoTemp: 1 inVectorAt: 0 47 <7D> blockReturn 48 <7C> returnTop
The closure [smallObject := 1] now, even though not referencing the largeObject, references the tempVector. This tempVector holds on the to large object --> memory is wasted that is not needed.
Back when implementing the SemChecker, I was thinking about this and wondering⦠for sure this can be optimized by allocating multiple arrays per scope, depending on how they are used later. But even then I am sure you can construct strange cases (multiple nested blocks).
And of course allocating 2 Arrays (or more) wastes memory, too.
So this is not a simple problem⦠the question is how much it matters in practice, though.
Marcus
Thanks for the explanation. I am also not sure if this would be a real problem in practice. I guess most code can be rewritten to avoid it. But it is probably a good idea to document this, maybe in a unit test. And who knows, maybe one day we'll have to revisit this. Sven
Am 29.06.2013 um 11:12 schrieb Marcus Denker <marcus.denker@inria.fr>:
OK, that seems OK.
The reason I was asking was because I wanted to know if this JavaScript bug would hit us as well:
http://point.davidglasser.net/2013/06/27/surprising-javascript-memory-leak.h...
It is slightly more complicated than the basic question of closing over an unreferenced variable though.
I just can't figure out a way to quickly test it in Pharo. I think it is worth looking into.
Yes, we have exactly the same problem.
imagine a method like this:
test | bigObject smallObject |
[bigObject := Array new: 1000000] value. ^[smallObject := 1].
both big and small are escaping variables that are written. This means they are allocated in an Array on the heap (Cog is calling this a TempVector).
There is just one tempVector created per scope, so we will have a tempVector with both smallObject and bigObject inside created for the method. This then is passed to the blocks, which access the temps via the vector.
The bytecode looks like this:
21 <8A 02> push: (Array new: 2) 23 <68> popIntoTemp: 0 24 <10> pushTemp: 0 25 <8F 10 00 07> closureNumCopied: 1 numArgs: 0 bytes 29 to 35 29 <40> pushLit: Array 30 <21> pushConstant: 1000000 31 <CD> send: new: 32 <8D 00 00> storeIntoTemp: 0 inVectorAt: 0 35 <7D> blockReturn 36 <C9> send: value 37 <87> pop 38 <10> pushTemp: 0 39 <8F 10 00 05> closureNumCopied: 1 numArgs: 0 bytes 43 to 47 43 <76> pushConstant: 1 44 <8D 01 00> storeIntoTemp: 1 inVectorAt: 0 47 <7D> blockReturn 48 <7C> returnTop
The closure [smallObject := 1] now, even though not referencing the largeObject, references the tempVector. This tempVector holds on the to large object --> memory is wasted that is not needed.
Back when implementing the SemChecker, I was thinking about this and wondering⦠for sure this can be optimized by allocating multiple arrays per scope, depending on how they are used later. But even then I am sure you can construct strange cases (multiple nested blocks).
And of course allocating 2 Arrays (or more) wastes memory, too.
So this is not a simple problem⦠the question is how much it matters in practice, though.
Thanks. So please let us separate problems. In the example from Sven there are two problems: - increase of memory per function invocation. That problem we don't have - easily keeping a reference to memory preventing it from being garbage collected. That is what you are talking about. And to me there is nothing wrong with it Norbert
On Jun 29, 2013, at 11:47 AM, Norbert Hartl <norbert@hartl.name> wrote:
Thanks. So please let us separate problems. In the example from Sven there are two problems:
- increase of memory per function invocation. That problem we don't have
Yes, you are right. I am not really knowledgeable about JavaScript⦠especially the strange stuff in there. ;-)
- easily keeping a reference to memory preventing it from being garbage collected. That is what you are talking about. And to me there is nothing wrong with it
Yes. It could lead to an object not being GCed, but this should not be problem in practice. Marcus
Am 28.06.2013 um 23:09 schrieb Sven Van Caekenberghe <sven@stfx.eu>:
On 28 Jun 2013, at 22:42, Marcus Denker <marcus.denker@inria.fr> wrote:
On Jun 28, 2013, at 9:26 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
Hi,
Does the closure returned from
| foo | foo := #( data 1 2 3 ). [ :x | x + 1 ]
close over foo ? No.
Since foo is not used, it should not, right ? right. if we get the AST of your method
ast := (TT>>#tt) ast. (ast scope lookupVar: 'foo') isEscaping
--> false.
OK, that seems OK.
The reason I was asking was because I wanted to know if this JavaScript bug would hit us as well:
http://point.davidglasser.net/2013/06/27/surprising-javascript-memory-leak.h...
It is slightly more complicated than the basic question of closing over an unreferenced variable though.
I just can't figure out a way to quickly test it in Pharo. I think it is worth looking into.
I don't think it is easy to do in pharo. The problem in the article only works because a string primitive is directly referenced. Primitives are copied as a whole when assigned. An object holding that huge string wouldn't be a problem because the enclosed activation record would just hold a reference to that object. As we don't have primitive types in smalltalk the problem doesn't apply IMHO. So his example is the combination that on every function invocation an activation record is created and the primitive string is copied into. The chain of the calling activation records _is_ the lexical scope IIRC. So tweaking the enclosed environment would need to copy and alter the activation records. And to be honest I fail to see why having foo in the scope is wrong. I mean why is it an error if it is there? In my opinion it is normal that foo is there but it can be an optimization to get rid of it, no? And I'm not sure it is easy in most cases to determine the liveliness of a variable. Norbert
Now the question is what the inspector of the context should show?
e.g.
tt | foo | foo := #( data 1 2 3 ). ^[ :x | x + 1. thisContext inspect ]
if we now execute:
TT new tt value: 2
the inspector pops up and shows both x and foo.
The reason is that we need to decide what to show
1) the variable that is stored in the context 2) all *possible* variables that I can write in the block and recompile and it works.
1) is this: ast statements last value scope localTempNames
2) is this:
ast statements last value scope allTemps
Opal implements is like this because the old Compiler (DebuggerMethodMap) did it like this. At first I thought this is wrong and I wanted to change it. But then I thought that it is not that wrong: in the debugger the inspector of the context shows not just the vars that are in the context, but all that *could* be there. All where I can lookup and get a result, e.g.
ast statements last value scope lookupVar: 'foo'
Thus all those that I *could* write in the code inside the block.
I am not sure, though⦠maybe we should change it.
Marcus
But it seems that the outer context of the block closure knows about foo, although the value seems nil.
And if it closes over foo, foo's value/binding won't be GC-ed so long as there is a reference to the block closure, even though foo is not practically accessible.
Sven
participants (3)
-
Marcus Denker -
Norbert Hartl -
Sven Van Caekenberghe