Hi,
So you're advocating for solution 1:
>
> 1) We change the instVarAt: / instVarAt:put: primitives comments in Pharo
> to match
> the slotAt: / slotAt:put: primitives behavior
I marked in bold the changes in the Pharo comment (so the comment now
reflects the actual behavior):
instVarAt: index
"Primitive. Answer a fixed variable in an object. The numbering of the
variables
corresponds to the named instance variables, followed by the indexed
instance
variables. Fail if the index is not an Integer or is not the index of a
fixed variable
*or indexed variable*. Essential. See Object documentation
whatIsAPrimitive."
<primitive: 173 error: ec>
self primitiveFailed
I'll make a patch later on.
On Tue, Nov 28, 2017 at 2:55 AM, David T. Lewis <[email protected]> wrote:
> On Mon, Nov 27, 2017 at 12:11:13PM -0800, Eliot Miranda wrote:
> > Hi Cl??ment,
> >
> > On Wed, Nov 22, 2017 at 5:35 AM, Cl??ment Bera <[email protected]>
> > wrote:
> >
> > > Hi All,
> > >
> > > It seems Pharo is lacking the slotAt: / slotAt:put: primitive and that
> the
> > > instVarAt: / instVarAt:put: primitive is not doing what it says it
> does.
> > > When I look at Object>>#instVarAt: I see:
> > >
> > > "[...]Answer a fixed variable in an object. The numbering of the
> variables
> > > corresponds to the named instance variables, followed by the indexed
> > > instance
> > > variables. Fail if the index is not an Integer or is not the index of a
> > > fixed variable[...]"
> > >
> > > Now in Pharo 7 when one does:
> > >
> > > #(iv1) instVarAt: 1, one gets #iv1.
> > >
> >
> > But the failure code should indeed answer #iv1.
> >
> >
> > > So the comment is wrong.
> > >
> > > The reason for this is that instVarAt: / instVarAt:put: should be
> > > primitive 73/74, while they are in Pharo 173/174 (primitive
> > > slotAt:/slotAt:put:).
> > >
> >
> > Why do you say "should"? The 173/174 primitives work well and avoid
> > unnecessary primitive failures (which is why they're there).
> >
> >
> > > The slot primitive access pointer slots of objects, whichever inst var
> or
> > > variable fields. A good example would be:
> > >
> > > | t |
> > > t := #temp.
> > > [ t ] instVarAt: 4, one gets #temp
> > >
> > > So I have two solutions for this problem:
> > > 1) We change the instVarAt: / instVarAt:put: primitives comments to
> match
> > > the slotAt: / slotAt:put: behavior, and in Pharo instVarAt: /
> > > instVarAt:put: means in factslotAt: / slotAt:put: .
> > >
> >
> > This is the performant solution, because...
> >
> >
> > > 2) We change the instVarAt: / instVarAt:put: primitives to primitive 73
> > > and 74, we introduce slotAt: / slotAt:put: with primitive 173/174 and
> we
> > > ask everyone to fix their code relying on instVarAt: / instVarAt:put:
> > > acting as slotAt: / slotAt:put:
> > >
> >
> > Early in Squeak Spur testing Chris Muller noticed a slow down in his code
> > He was accessing an Array using instVarAt:, which was failing because he
> > was using it to access an indexed field in an Array. Now in Spur the VM
> > must scan the object graph of a primitive's receiver and arguments to
> some
> > depth on primitive failure to follow forwarders. Because a) at that time
> > the VM computed a depth > 0 for instVarAt:[put:] it scanned the whole
> array
> > on each access, and b) the Array was large, this meant that the failures
> > were very expensive. So I added the slot primitives, since the old 73/74
> > primitive pair were only used in the instVarAt:[put:] methods that
> accessed
> > indexable slots on failure. So the new 173/174 pair subsumed the 73/74
> > pair.
> >
> > I eventually added an accessorDepth: pragma rot specify the accessor
> depth
> > manually when the automatically computed depth is too deep. It is for
> > at:[put:] because the String version of the at: 60 primitive appears to
> > access the innards of non-immediate Characters (it doesn't do this in
> Spur,
> > but the depth inferencer isn't clever enough to notice). So primitiveAt
> > looks like:
> >
> > primitiveAt
> > <accessorDepth: 0>
> > self commonAt: false
> >
> > So we could go back to using 73/74 for instVarAt:[put:] provided that
> their
> > accessorDepth ends up being 0, which likely means they will require the
> > pragma. But I think the simplest thing is to keep using 173/174. Here's
> > the Squeak code:
> >
> > Object>>instVarAt: index
> > "Primitive. Answer a fixed variable in an object. The numbering of the
> > variables
> > corresponds to the named instance variables, followed by the indexed
> > instance
> > variables. Fail if the index is not an Integer or is not the index of a
> > fixed or
> > indexed variable. Essential. See Object documentation whatIsAPrimitive."
> >
> > <primitive: 173 error: ec>
> > self primitiveFailed
> >
> > Object>>instVarAt: index put: anObject
> > "Primitive. Store a value into a fixed variable in an object. The
> numbering
> > of the
> > variables corresponds to the named instance variables, followed by the
> > indexed
> > instance variables. Fail if the index is not an Integer or is not the
> index
> > of a fixed
> > or indexed variable. Essential. See Object documentation
> whatIsAPrimitive."
> >
> > <primitive: 174 error: ec>
> > self primitiveFailed
> >
> > I've attached these methods.
> >
> >
> > What do you think ?
> > >
> >
> > I think for Spur it makes sense to use 173/174.
> >
>
> +1
>
> The 73/74 primitives (primitiveInstVarAt and primitiveInstVarAtPut) are
> semantically
> different from the 173/174 primitives (primitiveSlotAt and
> primitiveSlotAtPut).
>
> The 173/174 primitives are preferred for Spur images, and the 73/74
> primitives are
> used for V3 images. The VM can support both, and the image should call the
> primitive
> that it needs or wants to use. In any case, the primitive table numbering
> should not
> be confused between them.
>
> Squeak has the correct implementation for calling the primitives from a
> Spur image.
> See Cuis has an implementation of the image calling the appropriate
> primitive for
> whatever image format it happens to be using.
>
> Dave
>
>
>
--
Clément Béra
Pharo consortium engineer
https://clementbera.wordpress.com/
Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
