Bug in #pointsTo: ?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
33 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Bug in #pointsTo: ?

Mariano Martinez Peck
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

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
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
>

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Stéphane Ducasse

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


Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
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
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck
In reply to this post by Stéphane Ducasse


On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse <[hidden email]> 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
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

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:

> On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse
> <[hidden email]>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
> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck


2012/1/6 Levente Uzonyi <[hidden email]>
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:

On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse
<[hidden email]>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
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



--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:

> 2012/1/6 Levente Uzonyi <[hidden email]>
>
>> On Fri, 6 Jan 2012, Mariano Martinez Peck wrote:
>>
>>  On Fri, Jan 6, 2012 at 2:59 PM, Stéphane Ducasse
>>> <[hidden email]>**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-why-it-fixes-td4244097.html>
>>> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck


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>




--
Mariano
http://marianopeck.wordpress.com



--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
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
>

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck


On Sat, Jan 7, 2012 at 4:04 PM, Levente Uzonyi <[hidden email]> 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>>






--
Mariano
http://marianopeck.wordpress.com





--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
On Sat, 7 Jan 2012, Mariano Martinez Peck wrote:

> On Sat, Jan 7, 2012 at 4:04 PM, Levente Uzonyi <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck
In reply to this post by Levente Uzonyi-2
 

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



--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck
In reply to this post by Levente Uzonyi-2

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>









--
Mariano
http://marianopeck.wordpress.com





--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
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
>

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
In reply to this post by Mariano Martinez Peck
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
>

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck
In reply to this post by Levente Uzonyi-2
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 <[hidden email]> 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>>






 --





--
Mariano
http://marianopeck.wordpress.com





--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Mariano Martinez Peck


On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <[hidden email]> 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 <[hidden email]> 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





--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Bug in #pointsTo: ?

Levente Uzonyi-2
On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:

> On Mon, Jan 9, 2012 at 7:13 PM, Levente Uzonyi <[hidden email]> 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 <[hidden email]> 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
>

12