[Pharo-project] [squeak-dev] Re: Issue 4538 and CompiledMethod equality

Eliot Miranda eliot.miranda at gmail.com
Thu Sep 15 22:20:22 CEST 2011


On Thu, Sep 15, 2011 at 1:00 PM, Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> wrote:

> 2011/9/15 Eliot Miranda <eliot.miranda at gmail.com>:
> >
> >
> > On Thu, Sep 15, 2011 at 9:45 AM, Mariano Martinez Peck
> > <marianopeck at gmail.com> wrote:
> >>
> >> Ok, I understand. But
> >>
> >> On Thu, Sep 15, 2011 at 6:05 PM, Henrik Sperre Johansen
> >> <henrik.s.johansen at veloxit.no> wrote:
> >>>
> >>> On 15.09.2011 17:27, Mariano Martinez Peck wrote:
> >>>
> >>> Hi guys. I am having a problem with the integration of issue 4538:
> >>> http://code.google.com/p/pharo/issues/detail?id=4538
> >>>
> >>> I am serializing CompiledMethods with Fuel and then I materialize them.
> >>> To test they are correct, I use #=
> >>> So far it was working correctly, but now for the materialized method it
> >>> says they are not equal. The problem is that #= is failing in
> >>>
> >>> (self sameLiteralsAs: aCompiledMethod)
> >>>
> >>> And inside that method, it is failing because     (literal1 == literal2
> >>> or: [ literal1 literalEqual: literal2 ])
> >>> answers false.
> >>>
> >>> So... why literal1 literalEqual: literal2 answers false?  from what I
> can
> >>> say, having that selector: "literalEqual", then both MUST be equal,
> because
> >>> they are.
> >>> The problem is that it was added:
> >>>
> >>> Association >> literalEqual: otherLiteral
> >>>     "Answer true if the receiver and otherLiteral represent the same
> >>> literal.
> >>>     Variable bindings are literally equals only if identical.
> >>>     This is how variable sharing works, by preserving identity and
> >>> changing only the value."
> >>>     ^self == otherLiteral
> >>>
> >>>
> >>> So....I am not sure I agree with this change.
> >>>
> >>> Any idea how can we deal with this problem?
> >>>
> >>> Cheers
> >>>
> >>> --
> >>> Mariano
> >>> http://marianopeck.wordpress.com
> >>>
> >>> The change is correct, here's an example:
> >>>
> >>> Create a global:
> >>> Global := 1
> >>>
> >>> Create a method:
> >>>    foo
> >>>     ^Global
> >>>
> >>> Serialize method, deserialize
> >>>
> >>> Change value of global:
> >>> Global := 5.
> >>>
> >>> foo should return 5, not 1.
> >>> Unless it's actually the same association as in the SystemDictionary,
> >>> this will not be true.
> >>>
> >>
> >> I understand that. In fact, in Fuel we use exactly the same association
> of
> >> SystemDictionary for globals. Look this test example:
> >>
> >> testGlobalVariableMethod
> >>
> >>     | materializedCompiledMethod |
> >>     Smalltalk globals at: #TestGlobalVariableMethod2 put: false.
> >>     (self class compileSilently: 'globalVariableForTestingMethod
> >>     Transcript name.
> >>     ^ GlobalVariableForTesting.').
> >>
> >>     materializedCompiledMethod := self materializedCompiledMethod: (self
> >> class >> #globalVariableForTestingMethod).
> >>     Smalltalk globals at: #GlobalVariableForTesting put: true.
> >>     self assert:  (materializedCompiledMethod valueWithReceiver: self
> >> arguments: #()).
> >>
> >>
> >>
> >> BUT, it doesn't mean that Association is always used for globals.
> >> CompiledMethod equality is failing because of the last literal, the one
> that
> >> maps class name (symbol) and point to the real class. So...when I
> >> materialize, both CMs have non-identical associations for the last
> literal,
> >> but equal.
> >
> > As Henrik says the last literals are ideally #== to each other.  However,
> no
> > Squeak dialect makes any attempt to keep the class0side associations
> equal.
> > Look at a class-side method and you'll see it's last literal is
> > nil->SomeClass class.  Now since this association doesn't exist in
> Smalltalk
> > (unlike last literals on the instance side) the compiler merely creates
> > distinct ones for each class-side method.
> > Personally I don't think one can defend the position where method
> equality
> > is different for instance-side or class-side methods so there must be
> some
> > solutions:
>
> Hmm, good catch.
> A metaclass is never accessed by dictionary lookup, but only by
> sending #class to a class, so there is no point in maintaining a
> unique Association.
> (otherwise, we could maintain such Association in inst. var. thisClass).
> Having a nil key is a clear indication that lookup is pointless.
>
> The question is why having an association at all in the CompiledMethod ?
> For handling super sends ?
>

Yes, but two reasons.  One is super sends and the other is being able to
answer the methodClass (e.g. for CompiledMethod>>printOn: and for the
debugger).  One doesn't need to use an association, but one can't change
that without changing al the VMs /and/ changing the ClassBuilder so that
when it becomes a class on class redefinition the methodsa are updated.
 Indeed VisualWorks does exactly this.

In any case, the super send implementation in the VM means we can't change
this overnight :)



> I think a simple reference to the class would be enough.
> IMHO, the purpose was to simplify implementation.
>
> And I don't think the example of Henrik is worth :
> Lets just change it a bit:
>
> Object subclass: #SuperFoo.!
> Object subclass: #Bar.!
> SuperFoo subclass: #Foo.!
>
> SuperFoo compile: 'bar ^1'.
> Foo compile: 'bar
>   ^super bar *2'.
> foo := Foo new.
> Smalltalk at: #Foo put: Bar.
> ^foo bar
>
> Could you predict the result (will it try to invoke super Bar bar) ?
> Yes, since the last association is shared, we just broke (foo
> class>>bar) for no reason...
>

Right.  But Rube Goldberg machines work like Rube Goldberg machines.  Tis
the nature of the beast.  Doctors say "don't do that", and in Smalltalk we
should say "what did you expect"? :)  Being able to change the system is
worth the cost of it behaving contrary to common-sense.  Just like the
physical world :)


> Nicolas
>
> > 1. special case comparison of the last literal (the methodClass literal),
> > comparing keys and insisting that the keys be #== and the values be #==
> > (can't just define it as keys #== since all similar class-side methods
> will
> > be equal irrespective of their actual class).
> > 2. special case comparison of the last literal (the methodClass literal),
> > insisting only that the class of the literal be the same if it
> > isVariableBinding.
> > 3. make the compile unique class-side methodClass literals.  i.e. if a
> class
> > already has a class-side method then the compiler or method dictionary
> > insertion code must find that existing association and reuse it
> > Other ideas?
> >
> >>
> >> >From my point of view, that literal, the last one does not need to be
> >> identical to assume 2 CMs are equal. They just need to be equal.
> >>
> >>
> >> --
> >> Mariano
> >> http://marianopeck.wordpress.com
> >>
> >
> >
> >
> > --
> > best,
> > Eliot
> >
> >
> >
> >
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gforge.inria.fr/pipermail/pharo-project/attachments/20110915/6caa3845/attachment.htm>


More information about the Pharo-project mailing list