[Pharo-project] Bug in #pointsTo: ?
Hi guys. Levente recommended us to integrate the new version of #pointsTo:, and I did it. But not I think I found a bug. The method is: pointsTo: anObject "Answers true if I hold a reference to anObject, or false otherwise. Or stated another way: Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise" ^ (self instVarsInclude: anObject) or: [self class == anObject] The problem is that if the receiver is an instance of a compact class, this method answers true to: 'aaa' pointsTo: ByteString and I think that's incorrect. Moreoever, you can read the comment of the method "Or stated another way: Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise" That's not true for compact classes. With this change: pointsTo: anObject "Answers true if I hold a reference to anObject, or false otherwise. Or stated another way: Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise" ^ (self instVarsInclude: anObject) or: [(self class indexIfCompact = 0) and: [self class == anObject] ] it works fine. Do you agree with the change? cheers -- Mariano http://marianopeck.wordpress.com
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
Hi guys. Levente recommended us to integrate the new version of #pointsTo:, and I did it. But not I think I found a bug. The method is:
pointsTo: anObject "Answers true if I hold a reference to anObject, or false otherwise. Or stated another way:
Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
^ (self instVarsInclude: anObject) or: [self class == anObject]
The problem is that if the receiver is an instance of a compact class, this method answers true to: 'aaa' pointsTo: ByteString and I think that's incorrect. Moreoever, you can read the comment of the method "Or stated another way: Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise" That's not true for compact classes.
With this change:
pointsTo: anObject "Answers true if I hold a reference to anObject, or false otherwise. Or stated another way:
Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
^ (self instVarsInclude: anObject) or: [(self class indexIfCompact = 0) and: [self class == anObject] ]
it works fine.
Do you agree with the change?
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)). Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false. Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this method... Levente
cheers
-- Mariano http://marianopeck.wordpress.com
On Jan 6, 2012, at 2:46 PM, Levente Uzonyi wrote:
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)). Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false. Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this methodâ¦
Do you mean what I understand :)? that some tools compiled methods? :) Stef
On Fri, 6 Jan 2012, Stéphane Ducasse wrote:
On Jan 6, 2012, at 2:46 PM, Levente Uzonyi wrote:
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)). Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false. Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this method?
Do you mean what I understand :)? that some tools compiled methods? :)
No, they implemented more or less the same thing what #pointsTo: does (or should do). Levente
Stef
On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse <stephane.ducasse@inria.fr>wrote:
On Jan 6, 2012, at 2:46 PM, Levente Uzonyi wrote:
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)).
Ok, makes sense.
Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false.
Ok.
Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this methodâ¦
I found a problem with the PointerFinder: http://forum.world.st/Help-pointersTo-broken-and-fix-but-I-don-t-understand-... However, I still don't understand why it works if I use #instVarsInclude: and doesn't with #pointsTo: Do you mean what I understand :)? that some tools compiled methods? :)
Stef
-- Mariano http://marianopeck.wordpress.com
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse <stephane.ducasse@inria.fr>wrote:
On Jan 6, 2012, at 2:46 PM, Levente Uzonyi wrote:
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)).
Ok, makes sense.
Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false.
Ok.
Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this method?
I found a problem with the PointerFinder: http://forum.world.st/Help-pointersTo-broken-and-fix-but-I-don-t-understand-... However, I still don't understand why it works if I use #instVarsInclude: and doesn't with #pointsTo:
IIUC your problem is that MethodContext instances show up as objects which point to your object. And in fact (i think, but didn't check) they do point to your object, because the sender of those MethodContexts is the Date object. I think it only depends on when GC will happen. If the GC kicks in too late, then PointerFinder will find those transient objects and they will be listed. Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
-- Mariano http://marianopeck.wordpress.com
2012/1/6 Levente Uzonyi <leves@elte.hu>
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse
<stephane.ducasse@inria.fr>**wrote:
On Jan 6, 2012, at 2:46 PM, Levente Uzonyi wrote:
Of course, but I'd check for the class equality first, because it's
faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)).
Ok, makes sense.
Another thing that you should add (besides tests of course) is
SmallInteger >> #pointsTo: which should return false.
Ok.
Also make sure that pointer tracing tools use #pointsTo:. In Squeak I
found that some of them "reinvented" this method?
I found a problem with the PointerFinder: http://forum.world.st/Help-**pointersTo-broken-and-fix-but-** I-don-t-understand-why-it-**fixes-td4244097.html<http://forum.world.st/Help-pointersTo-broken-and-fix-but-I-don-t-understand-...> However, I still don't understand why it works if I use #instVarsInclude: and doesn't with #pointsTo:
IIUC your problem is that MethodContext instances show up as objects which point to your object.
yes
And in fact (i think, but didn't check) they do point to your object, because the sender of those MethodContexts is the Date object.
probably
I think it only depends on when GC will happen. If the GC kicks in too late, then PointerFinder will find those transient objects and they will be listed.
Thanks Levente. However, #pointersToExcept: does a GC at the beginning... so.. am I missing something? Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
2012/1/6 Levente Uzonyi <leves@elte.hu>
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse
<stephane.ducasse@inria.fr>**wrote:
On Jan 6, 2012, at 2:46 PM, Levente Uzonyi wrote:
Of course, but I'd check for the class equality first, because it's
faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)).
Ok, makes sense.
Another thing that you should add (besides tests of course) is
SmallInteger >> #pointsTo: which should return false.
Ok.
Also make sure that pointer tracing tools use #pointsTo:. In Squeak I
found that some of them "reinvented" this method?
I found a problem with the PointerFinder: http://forum.world.st/Help-**pointersTo-broken-and-fix-but-** I-don-t-understand-why-it-**fixes-td4244097.html<http://forum.world.st/Help-pointersTo-broken-and-fix-but-I-don-t-understand-...> However, I still don't understand why it works if I use #instVarsInclude: and doesn't with #pointsTo:
IIUC your problem is that MethodContext instances show up as objects which point to your object.
yes
And in fact (i think, but didn't check) they do point to your object, because the sender of those MethodContexts is the Date object.
probably
I think it only depends on when GC will happen. If the GC kicks in too late, then PointerFinder will find those transient objects and they will be listed.
Thanks Levente. However, #pointersToExcept: does a GC at the beginning... so.. am I missing something?
Yes, at the beginning The MethodContexts are created after that GC. Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
Thanks Levente. However, #pointersToExcept: does a GC at the beginning... so.. am I missing something?
Yes, at the beginning The MethodContexts are created after that GC.
But those MC shouldn't be excluded by the instVar objectsToAlwaysExclude ? objectsToAlwaysExclude := { results collector. thisContext. thisContext sender. thisContext sender sender. objectsToExclude. }.
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
Thanks Levente. However, #pointersToExcept: does a GC at the beginning... so.. am I missing something?
Yes, at the beginning The MethodContexts are created after that GC.
But those MC shouldn't be excluded by the instVar objectsToAlwaysExclude ?
objectsToAlwaysExclude := { results collector. thisContext. thisContext sender. thisContext sender sender. objectsToExclude. }.
Since #pointsTo: is not a primitive anymore, it will create at least one new MethodContext which is not included in that list. Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Sat, Jan 7, 2012 at 4:04 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
Thanks Levente. However, #pointersToExcept: does a GC at the
beginning... so.. am I missing something?
Yes, at the beginning The MethodContexts are created after that GC.
But those MC shouldn't be excluded by the instVar objectsToAlwaysExclude ?
objectsToAlwaysExclude := { results collector. thisContext. thisContext sender. thisContext sender sender. objectsToExclude. }.
Since #pointsTo: is not a primitive anymore,
That's an interesting point. Now I understand this better. Even if I didn't know the reasons (now I do), I even tried adding some more senders but still they appear :( I tried (for example) objectsToAlwaysExclude := { results collector. thisContext. thisContext sender. thisContext sender sender. thisContext sender sender sender. thisContext sender sender sender sender. thisContext sender sender sender sender sender. objectsToExclude. }. What I don't understand is why in Squeak it does work. Thanks in advance Levente!
it will create at least one new MethodContext which is not included in that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Sat, 7 Jan 2012, Mariano Martinez Peck wrote:
On Sat, Jan 7, 2012 at 4:04 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
Thanks Levente. However, #pointersToExcept: does a GC at the
beginning... so.. am I missing something?
Yes, at the beginning The MethodContexts are created after that GC.
But those MC shouldn't be excluded by the instVar objectsToAlwaysExclude ?
objectsToAlwaysExclude := { results collector. thisContext. thisContext sender. thisContext sender sender. objectsToExclude. }.
Since #pointsTo: is not a primitive anymore,
That's an interesting point. Now I understand this better. Even if I didn't know the reasons (now I do), I even tried adding some more senders but still they appear :( I tried (for example)
objectsToAlwaysExclude := { results collector. thisContext. thisContext sender. thisContext sender sender. thisContext sender sender sender. thisContext sender sender sender sender. thisContext sender sender sender sender sender. objectsToExclude. }.
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper than I should have, so I'll publish a few changes soon. Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using #instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo. Thanks Levente for your help. If you find something let us know, I want to learn :) Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using #instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to learn :)
I pushed my changes to the Squeak Inbox, which fully works around this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them. Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
Hi Levente. Thanks for looking into the issue. I saw your code and there is something I don't understand. pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise" (self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ] I don't understand the loop of 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. In which scenario can (self instVarsInclude: anObject) answer true, but the loop false? doesn't #instVarsInclude do exactly what you are doing there? Anyway, I have integrated your changes in Pharo, but still, I have the same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails. | a | 10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ] Thanks a lot, On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote: On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
Hi Levente. Thanks for looking into the issue. I saw your code and there is something I don't understand.
pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
I don't understand the loop of
1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
In which scenario can (self instVarsInclude: anObject) answer true, but the loop false?
The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots.
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold the reference to the argument, therefore this loop must be "repeated".
Anyway, I have integrated your changes in Pharo, but still, I have the same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object. Levente
| a | 10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
Hi Levente. Thanks for looking into the issue. I saw your code and there
is something I don't understand.
pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
I don't understand the loop of
1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
In which scenario can (self instVarsInclude: anObject) answer true, but the loop false?
The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots.
Ok, I see. And moreover, there has to be a GC in the middle, right? so..the scenario is "The scenario happens when the receiver has weak slots, the argument is referenced from one of those weak slots, but not from the other non-weak slots, and also when a GC runs between the invokation to #instVarsInclude: and the loop". is that correct? so that I will add this comment to the code ;)
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now PointerFinderTest are green :)
Levente
| a | 10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper
than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress.************com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.****com< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
Hi Levente. Thanks for looking into the issue. I saw your code and there
is something I don't understand.
pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
I don't understand the loop of
1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
In which scenario can (self instVarsInclude: anObject) answer true, but the loop false?
The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots.
Ok, I see. And moreover, there has to be a GC in the middle, right? so..the scenario is "The scenario happens when the receiver has weak slots, the argument is referenced from one of those weak slots, but not from the other non-weak slots, and also when a GC runs between the invokation to #instVarsInclude: and the loop". is that correct? so that I will add this comment to the code ;)
No, there doesn't have to be a GC. Levente
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now PointerFinderTest are green :)
Levente
| a | 10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper
than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress.************com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.****com< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Mon, Jan 9, 2012 at 8:01 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
Hi Levente. Thanks for looking into the issue. I saw your code and there
is something I don't understand.
pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
I don't understand the loop of
1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
In which scenario can (self instVarsInclude: anObject) answer true, but the loop false?
The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots.
Ok, I see. And moreover, there has to be a GC in the middle, right?
so..the scenario is "The scenario happens when the receiver has weak slots, the argument is referenced from one of those weak slots, but not from the other non-weak slots, and also when a GC runs between the invokation to #instVarsInclude: and the loop". is that correct? so that I will add this comment to the code ;)
No, there doesn't have to be a GC.
So..I am puzzle again. I said "In which scenario can (self instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ? thanks
Levente
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the
same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now
PointerFinderTest are green :)
Levente
| a |
10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper
than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the
problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other
methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around
this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.******com< http://marianopeck.**wordpress**.com <http://wordpress.com>< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.************com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.****com< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.**********com < http://marianopeck.wordpress.
com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
On Mon, Jan 9, 2012 at 8:01 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
Hi Levente. Thanks for looking into the issue. I saw your code and there
is something I don't understand.
pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
I don't understand the loop of
1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
In which scenario can (self instVarsInclude: anObject) answer true, but the loop false?
The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots.
Ok, I see. And moreover, there has to be a GC in the middle, right?
so..the scenario is "The scenario happens when the receiver has weak slots, the argument is referenced from one of those weak slots, but not from the other non-weak slots, and also when a GC runs between the invokation to #instVarsInclude: and the loop". is that correct? so that I will add this comment to the code ;)
No, there doesn't have to be a GC.
So..I am puzzle again. I said "In which scenario can (self instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones. Levente
thanks
Levente
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the
same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now
PointerFinderTest are green :)
Levente
| a |
10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper
than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the
problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other
methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around
this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.******com< http://marianopeck.**wordpress**.com <http://wordpress.com>< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.************com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.****com< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.**********com < http://marianopeck.wordpress.
com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right! Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
Levente
thanks
Levente
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold
the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the
same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now
PointerFinderTest are green :)
Levente
| a |
10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug
deeper
than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the
problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other
methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around
this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.************com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.********com< http://marianopeck.****wordpress**.com < http://wordpress.com><
http://marianopeck.**wordpress**.com <http://wordpress.com> <http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.**************com < http://marianopeck.wordpress.
com <http://marianopeck.wordpress.**********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.******com< http://marianopeck.**wordpress**.com <http://wordpress.com>< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.************com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.****com< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing. Levente
Levente
thanks
Levente
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold
the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the
same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now
PointerFinderTest are green :)
Levente
| a |
10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug
deeper
than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using
#instVarsInclude: rather than #pointsTo. And that solves the
problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other
methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to
learn :)
I pushed my changes to the Squeak Inbox, which fully works around
this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano
http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.************com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.********com< http://marianopeck.****wordpress**.com < http://wordpress.com><
http://marianopeck.**wordpress**.com <http://wordpress.com> <http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.**************com < http://marianopeck.wordpress.
com <http://marianopeck.wordpress.**********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.******com< http://marianopeck.**wordpress**.com <http://wordpress.com>< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.************com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.********com< http://marianopeck.**** wordpress.com <http://marianopeck.wordpress.****com< http://marianopeck.**wordpress.com<http://marianopeck.wordpress.com>
--
Mariano
http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
--
Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho⦠Does an object having weak slots mean it no longer pointsTo: the objects in those slots? Sadly, I have no better succinct suggestions. :/ Also, what happens when an object holds its (non-compact) class in a weak slot? In other words, is: wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray). a valid test? Cheers, Henry
On Mon, Jan 9, 2012 at 12:38 PM, Henrik Johansen < henrik.s.johansen@veloxit.no> wrote:
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots
and
the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho⦠Does an object having weak slots mean it no longer pointsTo: the objects in those slots?
Sadly, I have no better succinct suggestions. :/
Also, what happens when an object holds its (non-compact) class in a weak slot?
In other words, is:
wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray).
a valid test?
It is vacuous, since WeakArray is referred to indirectly from the roots via Smalltalk and so will not go away. The following is not a valid test because there could be a GC in between the at:put: and the assert:. | o oh | wa := WeakArray new: 1. o := Object new. oh := o hash. wa at: 1 put: o. o := nil. self assert: (wa at: 1) hash = oh but 99 times out of a hundred it'll pass :) Weak arrays are tricky beasts. To me, it doesn't matter that when used on WeakArray pointsTo: or instVarsIncludes: or whatever produce results that may be invalid at some subsequent time. The same is true for e.g. a normal Array that could be updated in some other thread. It does mater that at the point when an inst var is queried these methods/primitives answer the right answer. Writing valid tests that verify these answers is another matter entirely. Don't let the perfect be the enemy of the good.
Cheers, Henry
-- best, Eliot
On Mon, Jan 9, 2012 at 12:47 PM, Eliot Miranda <eliot.miranda@gmail.com>wrote:
On Mon, Jan 9, 2012 at 12:38 PM, Henrik Johansen < henrik.s.johansen@veloxit.no> wrote:
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots
and
the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho⦠Does an object having weak slots mean it no longer pointsTo: the objects in those slots?
Sadly, I have no better succinct suggestions. :/
Also, what happens when an object holds its (non-compact) class in a weak slot?
In other words, is:
wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray).
a valid test?
It is vacuous, since WeakArray is referred to indirectly from the roots via Smalltalk and so will not go away. The following is not a valid test because there could be a GC in between the at:put: and the assert:.
| o oh | wa := WeakArray new: 1. o := Object new. oh := o hash. wa at: 1 put: o. o := nil. self assert: (wa at: 1) hash = oh
but 99 times out of a hundred it'll pass :)
and it can better be written | oh | wa := WeakArray new: 1. oh := (wa at: 1 put: Object new) hash. self assert: (wa at: 1) hash = oh
Weak arrays are tricky beasts. To me, it doesn't matter that when used on WeakArray pointsTo: or instVarsIncludes: or whatever produce results that may be invalid at some subsequent time. The same is true for e.g. a normal Array that could be updated in some other thread. It does mater that at the point when an inst var is queried these methods/primitives answer the right answer. Writing valid tests that verify these answers is another matter entirely. Don't let the perfect be the enemy of the good.
Cheers, Henry
-- best, Eliot
-- best, Eliot
Ok guys....what about the following: stronglyPointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise" (self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ] pointsTo: anObject "Answers true if I hold a reference to anObject, or false otherwise" ^ (self instVarsInclude: anObject) or: [ ^self class == anObject and: [ self class isCompact not ] ] And then, from the the tracing pointer stuff (such as #pointersToExcept:) we use #stronglyPointsTo do you agree? On Mon, Jan 9, 2012 at 9:55 PM, Eliot Miranda <eliot.miranda@gmail.com>wrote:
On Mon, Jan 9, 2012 at 12:47 PM, Eliot Miranda <eliot.miranda@gmail.com>wrote:
On Mon, Jan 9, 2012 at 12:38 PM, Henrik Johansen < henrik.s.johansen@veloxit.no> wrote:
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak
slots and
the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho⦠Does an object having weak slots mean it no longer pointsTo: the objects in those slots?
Sadly, I have no better succinct suggestions. :/
Also, what happens when an object holds its (non-compact) class in a weak slot?
In other words, is:
wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray).
a valid test?
It is vacuous, since WeakArray is referred to indirectly from the roots via Smalltalk and so will not go away. The following is not a valid test because there could be a GC in between the at:put: and the assert:.
| o oh | wa := WeakArray new: 1. o := Object new. oh := o hash. wa at: 1 put: o. o := nil. self assert: (wa at: 1) hash = oh
but 99 times out of a hundred it'll pass :)
and it can better be written
| oh | wa := WeakArray new: 1. oh := (wa at: 1 put: Object new) hash. self assert: (wa at: 1) hash = oh
Weak arrays are tricky beasts. To me, it doesn't matter that when used on WeakArray pointsTo: or instVarsIncludes: or whatever produce results that may be invalid at some subsequent time. The same is true for e.g. a normal Array that could be updated in some other thread. It does mater that at the point when an inst var is queried these methods/primitives answer the right answer. Writing valid tests that verify these answers is another matter entirely. Don't let the perfect be the enemy of the good.
Cheers, Henry
-- best, Eliot
-- best, Eliot
-- Mariano http://marianopeck.wordpress.com
On Mon, Jan 9, 2012 at 1:16 PM, Mariano Martinez Peck <marianopeck@gmail.com
wrote:
Ok guys....what about the following:
stronglyPointsTo: anObject
"Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
Named inst vars in weak arrays are strong references. So the above looks wrong to me. Its more like this: 1 to: self class instSize do: [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. self class isWeak ifFalse: [1 to: self basicSize do: [:i| (se;f basicAt: i) == anObject ifTrue: [ ^true ] ]. ^false This should be a primitive. I believe Levente's primitive is even more useful and equivalent to 1 to: self class instSize do: [:i| (self instVarAt: i) == anObject ifTrue: [ ^i ] ]. self class isWeak ifFalse: [1 to: self basicSize do: [:i| (se;f basicAt: i) == anObject ifTrue: [ ^i + self class instSize ] ]. ^0
pointsTo: anObject "Answers true if I hold a reference to anObject, or false otherwise"
^ (self instVarsInclude: anObject) or: [ ^self class == anObject and: [ self class isCompact not ] ]
And then, from the the tracing pointer stuff (such as #pointersToExcept:) we use #stronglyPointsTo
do you agree?
On Mon, Jan 9, 2012 at 9:55 PM, Eliot Miranda <eliot.miranda@gmail.com>wrote:
On Mon, Jan 9, 2012 at 12:47 PM, Eliot Miranda <eliot.miranda@gmail.com>wrote:
On Mon, Jan 9, 2012 at 12:38 PM, Henrik Johansen < henrik.s.johansen@veloxit.no> wrote:
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak
slots and
the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho⦠Does an object having weak slots mean it no longer pointsTo: the objects in those slots?
Sadly, I have no better succinct suggestions. :/
Also, what happens when an object holds its (non-compact) class in a weak slot?
In other words, is:
wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray).
a valid test?
It is vacuous, since WeakArray is referred to indirectly from the roots via Smalltalk and so will not go away. The following is not a valid test because there could be a GC in between the at:put: and the assert:.
| o oh | wa := WeakArray new: 1. o := Object new. oh := o hash. wa at: 1 put: o. o := nil. self assert: (wa at: 1) hash = oh
but 99 times out of a hundred it'll pass :)
and it can better be written
| oh | wa := WeakArray new: 1. oh := (wa at: 1 put: Object new) hash. self assert: (wa at: 1) hash = oh
Weak arrays are tricky beasts. To me, it doesn't matter that when used on WeakArray pointsTo: or instVarsIncludes: or whatever produce results that may be invalid at some subsequent time. The same is true for e.g. a normal Array that could be updated in some other thread. It does mater that at the point when an inst var is queried these methods/primitives answer the right answer. Writing valid tests that verify these answers is another matter entirely. Don't let the perfect be the enemy of the good.
Cheers, Henry
-- best, Eliot
-- best, Eliot
-- Mariano http://marianopeck.wordpress.com
-- best, Eliot
2012/1/9 Eliot Miranda <eliot.miranda@gmail.com>:
On Mon, Jan 9, 2012 at 1:16 PM, Mariano Martinez Peck <marianopeck@gmail.com> wrote:
Ok guys....what about the following:
stronglyPointsTo: anObject
   "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
   (self instVarsInclude: anObject)       ifTrue: [          self class isWeak ifFalse: [ ^true ].          1 to: self class instSize do: [ :i |             (self instVarAt: i) == anObject ifTrue: [ ^true ] ].          ^false ]       ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
Named inst vars in weak arrays are strong references. Â So the above looks wrong to me. Its more like this:
  1 to: self class instSize do:     [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ].   self class isWeak ifFalse:     [1 to: self basicSize do:       [:i| (se;f basicAt: i) == anObject ifTrue: [ ^true ] ].   ^false
But instVarIncludes: does walk over all the indexed slots, not just the inst vars (yeah, the name does not tell) 1 to: self class instSize do: [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. 1 to: self basicSize do: [:i| (self basicAt: i) == anObject ifTrue: [ ^true ] ]. ^false So I think this part of the method works... Only the class test looks strange, (WeakArray new) does point strongly to its class like Henrik mentioned with his (WeakArray with: WeakArray) joke - or I didn't understand ;) We could also add a self basicSize = 0 ifTrue: [^true] before any iteration, but better use new primitive indeed... Nicolas
This should be a primitive. Â I believe Levente's primitive is even more useful and equivalent to
  1 to: self class instSize do:     [:i| (self instVarAt: i) == anObject ifTrue: [ ^i ] ].   self class isWeak ifFalse:     [1 to: self basicSize do:       [:i| (se;f basicAt: i) == anObject ifTrue: [ ^i + self class instSize ] ].   ^0
pointsTo: anObject    "Answers true if I hold a reference to anObject, or false otherwise"
   ^ (self instVarsInclude: anObject)       or: [ ^self class == anObject and: [ self class isCompact not ] ]
And then, from the the tracing pointer stuff (such as #pointersToExcept:) we use #stronglyPointsTo
do you agree?
On Mon, Jan 9, 2012 at 9:55 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Mon, Jan 9, 2012 at 12:47 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Mon, Jan 9, 2012 at 12:38 PM, Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can   (self
instVarsInclude: anObject) Â answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. Â How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: Â I would guess weak references are also taken into account. Â So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? Â or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho⦠Does an object having weak slots mean it no longer pointsTo: the objects in those slots?
Sadly, I have no better succinct suggestions. :/
Also, what happens when an object holds its (non-compact) class in a weak slot?
In other words, is:
wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray).
a valid test?
It is vacuous, since WeakArray is referred to indirectly from the roots via Smalltalk and so will not go away. Â The following is not a valid test because there could be a GC in between the at:put: and the assert:.
| o oh | wa := WeakArray new: 1. o := Object new. oh := o hash. wa at: 1 put: o. o := nil. self assert: (wa at: 1) hash = oh
but 99 times out of a hundred it'll pass :)
and it can better be written
| oh | wa := WeakArray new: 1. oh := (wa at: 1 put: Object new) hash. self assert: (wa at: 1) hash = oh
Weak arrays are tricky beasts. Â To me, it doesn't matter that when used on WeakArray pointsTo: or instVarsIncludes: or whatever produce results that may be invalid at some subsequent time. Â The same is true for e.g. a normal Array that could be updated in some other thread. Â It does mater that at the point when an inst var is queried these methods/primitives answer the right answer. Â Writing valid tests that verify these answers is another matter entirely. Â Don't let the perfect be the enemy of the good.
Cheers, Henry
-- best, Eliot
-- best, Eliot
-- Mariano http://marianopeck.wordpress.com
-- best, Eliot
On Jan 9, 2012, at 11:00 50PM, Nicolas Cellier wrote:
2012/1/9 Eliot Miranda <eliot.miranda@gmail.com>:
On Mon, Jan 9, 2012 at 1:16 PM, Mariano Martinez Peck <marianopeck@gmail.com> wrote:
Ok guys....what about the following:
stronglyPointsTo: anObject
"Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
Named inst vars in weak arrays are strong references. So the above looks wrong to me. Its more like this:
1 to: self class instSize do: [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. self class isWeak ifFalse: [1 to: self basicSize do: [:i| (se;f basicAt: i) == anObject ifTrue: [ ^true ] ]. ^false
But instVarIncludes: does walk over all the indexed slots, not just the inst vars (yeah, the name does not tell)
1 to: self class instSize do: [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. 1 to: self basicSize do: [:i| (self basicAt: i) == anObject ifTrue: [ ^true ] ]. ^false
So I think this part of the method works...
Only the class test looks strange, (WeakArray new) does point strongly to its class like Henrik mentioned with his (WeakArray with: WeakArray) joke - or I didn't understand ;)
Yes, that was my point, sorry for the murkiness with which I tried to make it. In Levente's code: anObject instVarsInclude: itsClass -> true class is weak -> true ref in weak slot -> true And all of a sudden, you don't pointTo: your class anymore just because you have it in a weak slot. (The test would fail) Cheers, Henry
On Mon, 9 Jan 2012, Henrik Johansen wrote:
On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
So..I am puzzle again. I said "In which scenario can (self
instVarsInclude: anObject) answer true, but the loop false? " you answered: "The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots." Imagine the receiver has a weak slot XXX that points to anObject. So (self instVarsInclude: anObject) answers true. How can the loop answer false without a GC? why would XXX stop pointing to anObject if there is not GC in the middle ?
The loop doesn't iterate over the indexable slots, only named ones.
grrr you are right!
Ok, so I finally got it. So previously pointersTo: would answer true even if the slots were weak. Now it would answer false, and that's why you have changed the method comment. Now I am thinking if this should be the default behavior of #pointsTo:. If I just read the selector #pointsTo: I would guess weak references are also taken into account. So that's why I am not completely sure. Aren't there any other impact of the system because of this change? what about having #stronglyPointsTo: with this new version and have another one #pointsTo: which considers weak also? does it make sense? or it is better to directly avoid weak by defualt in #pointsTo?
I wasn't sure about this, that's why it's not in Squeak yet. Ignoring weak references is okay as long as these methods are only used for pointer tracing.
Levente
Even with a good comment, the naming starts to make little sense, imho?
Since #pointsTo: doesn't use primitive 132 directly, it's only used from the pointer tracing tools, which shouldn't really follow weak references.
Does an object having weak slots mean it no longer pointsTo: the objects in those slots?
If you mean "points to", then no. :)
Sadly, I have no better succinct suggestions. :/
Also, what happens when an object holds its (non-compact) class in a weak slot?
In other words, is:
wa := WeakArray new: 1. wa at: 1 put: WeakArray. self assert: (wa pointsTo: WeakArray).
a valid test?
Yes, because its class pointer points to its class (this sounds funny :)). Levente
Cheers, Henry
mariano could you update the comments to reflect this discussion. I would like that the next guy that looks at the code can learn and not be puzzled. Stef On Jan 9, 2012, at 7:42 PM, Mariano Martinez Peck wrote:
On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <leves@elte.hu> wrote: On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
Hi Levente. Thanks for looking into the issue. I saw your code and there is something I don't understand.
pointsTo: anObject "Answers true if the garbage collector would fail to collect anObject because I hold a reference to it, or false otherwise"
(self instVarsInclude: anObject) ifTrue: [ self class isWeak ifFalse: [ ^true ]. 1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ]. ^false ] ifFalse: [ ^self class == anObject and: [ self class isCompact not ] ]
I don't understand the loop of
1 to: self class instSize do: [ :i | (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
In which scenario can (self instVarsInclude: anObject) answer true, but the loop false?
The scenario happens when the receiver has weak slots and the argument is referenced from one of those weak slots, but not from the other slots.
Ok, I see. And moreover, there has to be a GC in the middle, right? so..the scenario is "The scenario happens when the receiver has weak slots, the argument is referenced from one of those weak slots, but not from the other non-weak slots, and also when a GC runs between the invokation to #instVarsInclude: and the loop". is that correct? so that I will add this comment to the code ;)
doesn't #instVarsInclude do exactly what you are doing there?
Just partially. Since we have no information about which slots hold the reference to the argument, therefore this loop must be "repeated".
Ok, I see.
Anyway, I have integrated your changes in Pharo, but still, I have the same problem :( If I understand correctly, the following shouldn't fail, but it does. Here is the version of Squeak that fails.
The assetion fails, because the indirection vector (the Array found by PointerFinder) holds a reference to the object after the first assignment to a. If you move the temporary inside the block or use an inlined loop (e.g. #to:do: with literal block argument), then the assertion won't fail. So this is just a normal (maybe surprising) reference to the object.
Excellent. Now I got it. Thank you very much for your help Levente. Now PointerFinderTest are green :)
Levente
| a | 10 timesRepeat: [ a := Date new. Smalltalk garbageCollect. self assert: (PointerFinder pointersTo: a) isEmpty ]
Thanks a lot,
On Mon, Jan 9, 2012 at 2:40 PM, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
What I don't understand is why in Squeak it does work.
Because #pointsTo: is not used in Squeak (yet). As usual I dug deeper than I should have, so I'll publish a few changes soon.
Ok, you are right. Squeak #inboundPointersExcluding: is using #instVarsInclude: rather than #pointsTo. And that solves the problem in Pharo as well. But still, I would like to understand why we get those method contexts with #pointsTo.
Because #pointsTo: is a normal message send, it even sends other methods, so it will create contexts.
Thanks Levente for your help. If you find something let us know, I want to learn :)
I pushed my changes to the Squeak Inbox, which fully works around this issue. The changes about weak references can simply be removed if you don't like them, the rest will just work without them.
Levente
Thanks
Levente
Thanks in advance Levente!
it will create at least one new MethodContext which is not included in
that list.
Levente
Levente
Thanks again.
Levente
Do you mean what I understand :)? that some tools compiled methods? :)
Stef
--
Mariano http://marianopeck.wordpress.**********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.******com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
--
Mariano http://marianopeck.wordpress.********com < http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.****com<http://marianopeck.** wordpress.com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.******com <http://marianopeck.wordpress. **** com <http://marianopeck.wordpress.**com<http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.****com <http://marianopeck.wordpress.** com <http://marianopeck.wordpress.com>>
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)).
Are you sure Cog doesn't care? I think it does. From what I can see in Cog code the entry point to inline #class is from the bytecoodes. If you see initializeBytecodeTableForClosureV3 ... #(1 199 199 genSpecialSelectorClass needsFrameNever: notMapped 0). "not mapped because it is directly inlined (for now)" ... so if you use a the normal bytecode for #class then I think #class is not inlined in machine code. At least from my tests I noticed that. Cheers
Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false. Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this method...
Levente
cheers
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
On Sun, 8 Jan 2012, Mariano Martinez Peck wrote:
Of course, but I'd check for the class equality first, because it's faster (yeah, i know you turned off inlining of #class, but Cog doesn't care about that ;)).
Are you sure Cog doesn't care? I think it does. From what I can see in Cog code the entry point to inline #class is from the bytecoodes. If you see initializeBytecodeTableForClosureV3 ... #(1 199 199 genSpecialSelectorClass needsFrameNever: notMapped 0). "not mapped because it is directly inlined (for now)" ...
so if you use a the normal bytecode for #class then I think #class is not inlined in machine code. At least from my tests I noticed that.
I guess you're right. Levente
Cheers
Another thing that you should add (besides tests of course) is SmallInteger >> #pointsTo: which should return false. Also make sure that pointer tracing tools use #pointsTo:. In Squeak I found that some of them "reinvented" this method...
Levente
cheers
-- Mariano http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
-- Mariano http://marianopeck.wordpress.com
participants (6)
-
Eliot Miranda -
Henrik Johansen -
Levente Uzonyi -
Mariano Martinez Peck -
Nicolas Cellier -
Stéphane Ducasse