[Pharo-project] Problem with CompiledMethodTrailer

Henrik Sperre Johansen henrik.s.johansen at veloxit.no
Thu May 12 16:02:42 CEST 2011


On 10.05.2011 16:22, Igor Stasenko wrote:
> On 10 May 2011 13:33, Henrik Sperre Johansen
> <henrik.s.johansen at veloxit.no>  wrote:
>> On 10.05.2011 12:44, Igor Stasenko wrote:
>>> On 10 May 2011 11:27, Mariano Martinez Peck<marianopeck at gmail.com>    wrote:
>>>> On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko<siguctua at gmail.com>
>>>>   wrote:
>>>>> On 10 May 2011 09:48, Mariano Martinez Peck<marianopeck at gmail.com>
>>>>>   wrote:
>>>>>> On Tue, May 10, 2011 at 9:41 AM, Marcus Denker<marcus.denker at inria.fr>
>>>>>> wrote:
>>>>>>> On May 10, 2011, at 9:32 AM, Mariano Martinez Peck wrote:
>>>>>>>
>>>>>>>> what is even worst, is that even after removing those correct CM, and
>>>>>>>> doing a GC etc...they still don't disappear.
>>>>>>>>
>>>>>>> There is
>>>>>>>
>>>>>>>         ScriptLoader new cleanUpMethods
>>>>>>>
>>>>>>> This is called from #cleanUpForRelease and normally should make sure
>>>>>>> there
>>>>>>> are no old methods.
>>>>>>>
>>>>>> hehehehe if I do: ScriptLoader new cleanUpForRelease, then inspect
>>>>>> ((CompiledMethod allInstances select:  [:each | each trailer kind =
>>>>>> #VarLengthSourcePointer] ) )
>>>>>>
>>>>>> they are not GC'ed and if then I click on the first element in the
>>>>>> inspector.... VM CRASH!!!  with both, InterpreterVM and CogVM.
>>>>>>
>>>>>> :(
>>>>>>
>>>>> okay. it seems i found the offender. Its a CompiledMethod
>>>>> class>>cleanUp.
>>>>> It changing a source pointer of all non-installed methods to 0.
>>>> Good catch!!!  :)   Anyway, why CompiledMethod class>>cleanUp would like
>>>> to
>>>> destroy source pointers???
>>>> If I understood correctly (please correct me), all compiled methods will
>>>> loose the pointer to sources and hence they will be decompiled after when
>>>> they are ask their source!
>>> Yes, if you wipe the sourcePointer, there is no way how you can get a
>>> source code of method.
>> I believe the "not getting the source code" was on purpose.
>> It's done f.ex. as part of condenseChanges, as the source pointers would
>> have been invalid anyways.
>> The intent I believe is that you should get decompiled source if you inspect
>> old(but still referenced) CompiledMethods, instead of a bogus display of
>> some random part of the new changes file, and remove error handling in
>> source lookup where pointer could potentially be beyond end of file.
>>
>> This of course worked much faster before trailers, when setSourcePointer:
>> actually modified the instance inline, rather than become'ing a copy,
>> nowadays I agree with Igor that it'd probably be more conceptually clean to
>> the empty MethodTrailer in destroySourcePointer :)
>>
>> Just remember to make sure such instances decompile as expected to uphold
>> why the zapping was done in the first place.
>> If we're lucky,  it won't crash the VM either ;)
>>
> Actually , in #destroySourcePointer setting sourcePointer = 0 is pretty useless.
> Since now, we can embed source code in trailer, we can replace a
> senders of it with #dropSourcePointer.
> Then you don't have to decompile the method, even if its no longer
> installed and cannot be located in .sources or .changes files anymore.
You are forgetting, the send to #destroySourcePointer happens as a 
manual step AFTER sources have been truncated.
So, trying to move it's source to a trailer from the changefile will be 
a rather bad idea, as the source at its old pointer is either garbage, 
or simply out of bounds of the change file already.
> We could even do this at the method installation time:
>   - if a newly installed method replacing an old one,
> we tell old one to drop its source pointer immediately.
>
> Then, by definition, you cannot have non-installed methods with wrong
> source pointer, and all those methods will still be able to show you
> an original source code.
>
> Sounds like an idea?
This would be the alternative, yes.
Not sure if it's worth it, as the source will still be valid as long as 
change file isn't messed with.

Decompiled code for old methods which were moved as part of a 
truncation, where the new versions have the exact same, and correct 
source doesn't sound like the end of the world to me. The method itself 
did not actually change.

Cheers,
Henry



More information about the Pharo-project mailing list