use of pointsTo: for includesSelector:

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

use of pointsTo: for includesSelector:

Eliot Miranda-2
Hi All,

    I just got a little burned.  I was searching through an Array of symbols and integers (it is the PrimitiveTable from the VM), wanting to filter-out methods with a with a certain pragma.  The code looks like:

anArray doWithIndex:
[:entry :index|
(self whichClassIncludesSelector: entry) ifNotNil:
[:c| | m |
m := c >> entry.
(m pragmaAt: #option:) ifNotNil:
[:pragma|
(initializationOptions at: (pragma arguments first) ifAbsent: [true]) ifFalse:
[anArray at: index put: 0]]]]

the error was a keyNotFound error for c >> entry.  Turns out entry was the integer 306, a code for a quick primitive that returns some inst var.  The question is why did (self whichClassIncludesSelector: entry) evaluate to other than nil given that 306 is /not/ a selector in any of the classes from self on up.  Well, it's MethodDictionary's use of pointsTo: that is at fault:

MethodDictionary>>includesKey: aSymbol
"This override assumes that pointsTo is a fast primitive"

aSymbol ifNil: [^ false].
^ self pointsTo: aSymbol

ProtoObject>>pointsTo: anObject
"This method returns true if self contains a pointer to anObject,
and returns false otherwise"
<primitive: 132>
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

Turns out that 306 was the tally of one of the method dictionaries along self's superclass chain.  This seems to be to be completely bogus.  Do we really need crude performance hacks like this any more?
--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: use of pointsTo: for includesSelector:

Nicolas Cellier
Ouch! bad bug!
I don't know if we need the speed, but we need the correctness.

To add to weirdness, I tested that (Object canUnderstand: Object methodDictionary size) is true, but I have (Object canUnderstand: Object selectors size) answering false, because it has two non nil slot less than the tally...



2013/10/25 Eliot Miranda <[hidden email]>
Hi All,

    I just got a little burned.  I was searching through an Array of symbols and integers (it is the PrimitiveTable from the VM), wanting to filter-out methods with a with a certain pragma.  The code looks like:

anArray doWithIndex:
[:entry :index|
(self whichClassIncludesSelector: entry) ifNotNil:
[:c| | m |
m := c >> entry.
(m pragmaAt: #option:) ifNotNil:
[:pragma|
(initializationOptions at: (pragma arguments first) ifAbsent: [true]) ifFalse:
[anArray at: index put: 0]]]]

the error was a keyNotFound error for c >> entry.  Turns out entry was the integer 306, a code for a quick primitive that returns some inst var.  The question is why did (self whichClassIncludesSelector: entry) evaluate to other than nil given that 306 is /not/ a selector in any of the classes from self on up.  Well, it's MethodDictionary's use of pointsTo: that is at fault:

MethodDictionary>>includesKey: aSymbol
"This override assumes that pointsTo is a fast primitive"

aSymbol ifNil: [^ false].
^ self pointsTo: aSymbol

ProtoObject>>pointsTo: anObject
"This method returns true if self contains a pointer to anObject,
and returns false otherwise"
<primitive: 132>
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

Turns out that 306 was the tally of one of the method dictionaries along self's superclass chain.  This seems to be to be completely bogus.  Do we really need crude performance hacks like this any more?
--
best,
Eliot






Reply | Threaded
Open this post in threaded view
|

Re: use of pointsTo: for includesSelector:

David T. Lewis
In reply to this post by Eliot Miranda-2
On Fri, Oct 25, 2013 at 02:18:07PM -0700, Eliot Miranda wrote:

>
> ProtoObject>>pointsTo: anObject
> "This method returns true if self contains a pointer to anObject,
> and returns false otherwise"
> <primitive: 132>
> 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

The version of ProtoObject>>pointsTo: that you quote here has a time stamp
from 1999. The current version in Squeak has a time stamp of 2008, and
looks like this:


ProtoObject>>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]



Reply | Threaded
Open this post in threaded view
|

Re: use of pointsTo: for includesSelector:

Levente Uzonyi-2
On Fri, 25 Oct 2013, David T. Lewis wrote:

> On Fri, Oct 25, 2013 at 02:18:07PM -0700, Eliot Miranda wrote:
>>
>> ProtoObject>>pointsTo: anObject
>> "This method returns true if self contains a pointer to anObject,
>> and returns false otherwise"
>> <primitive: 132>
>> 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
>
> The version of ProtoObject>>pointsTo: that you quote here has a time stamp
> from 1999. The current version in Squeak has a time stamp of 2008, and
> looks like this:
>
>
> ProtoObject>>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 old #pointsTo: method was renamed to #instVarsInclude: somewhere
around Squeak 4.3. MethodDictionary used to use #instVarsInclude:, so the
bug was still present in the Trunk, but I just removed the #includesKey:
method in Kernel-ul.815, because we don't need the extra speed, nor the
bug. :)

At the same time I have uploaded a new version of Collections, which has a
slighly faster version of Dictionary >> #includesKey:.


Levente

Reply | Threaded
Open this post in threaded view
|

Re: use of pointsTo: for includesSelector:

timrowledge
In reply to this post by Nicolas Cellier

On 25-10-2013, at 2:53 PM, Nicolas Cellier <[hidden email]> wrote:

> Ouch! bad bug!
> I don't know if we need the speed, but we need the correctness.
A related bit of code is the Preferences class>thoroughSenders which really seems like something that doesn’t need to be a preference anymore. I’m pretty sure any machine we’re plausibly running on these days can handle the thorough search for senders; my guess is that the first generation Pi is about the slowest likely Squeakinator and it has no problems. Or is anyone still running a production system on a 100MHz 386?


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
"Bother" said Pooh, as he flunked the the sobriety test.