Re: [Pharo-project] Problem with CompiledMethodTrailer
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. -- Marcus Denker -- http://www.marcusdenker.de INRIA Lille -- Nord Europe. Team RMoD.
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker <marcus.denker@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. :(
-- Marcus Denker -- http://www.marcusdenker.de INRIA Lille -- Nord Europe. Team RMoD.
-- Mariano http://marianopeck.wordpress.com
On 10 May 2011 09:48, Mariano Martinez Peck <marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker <marcus.denker@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. When i do this, my image hangs. Now i thinking that it should actually use empty trailer for those methods i.e. CompiledMethodTrailer empty.. Still i found it strange why some methods has broken bytecode. My theory is that some nasty code somewhere copied a compiled methods using some unsafe technique, like: 1 to: oldMethod size do: [ :i | newMethod at: i put: (oldMethod at: i) ] Because when you constructing compiled method normally (by compiling them) , there is no way how you can get broken compiled methods. (we would discover it much much earlier, and in fact CompiledMethodTrailers are working for more than a year in both pharo and squeak, and there was no any issues like this from their side). -- Best regards, Igor Stasenko AKA sig.
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko <siguctua@gmail.com> wrote:
On 10 May 2011 09:48, Mariano Martinez Peck <marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker <marcus.denker@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! I must be wrong, because the dev image when it is build, it calls all cleanUp methods for classes, so CompiledMethod class >> cleanUp shuld have been called, but if I take the Pharo1.3 result, I can see the source code (not decompiled)
When i do this, my image hangs.
I have just tried and it crash!!! Actually, I evaluated CompiledMethod cleanUp. And just after cmd+b in CompiledMethod (to browse it) -> crash How it is possilbe it didn't crash while building the dev image? or the first time I browse a class? maybe this incorrect CM are created by the build process just after the cleanUp ?
Now i thinking that it should actually use empty trailer for those methods i.e. CompiledMethodTrailer empty..
Still i found it strange why some methods has broken bytecode.
Yes!! and why they are not GCed!!
My theory is that some nasty code somewhere copied a compiled methods using some unsafe technique, like:
1 to: oldMethod size do: [ :i | newMethod at: i put: (oldMethod at: i) ]
I will search to see if I find something. The thing is that it is reproducible. Each new dev image build from scratch in a core, has these guys.
Because when you constructing compiled method normally (by compiling them) , there is no way how you can get broken compiled methods. (we would discover it much much earlier,
yes, exactly.
and in fact CompiledMethodTrailers are working for more than a year in both pharo and squeak, and there was no any issues like this from their side).
+1
-- Best regards, Igor Stasenko AKA sig.
-- Mariano http://marianopeck.wordpress.com
On 10 May 2011 11:27, Mariano Martinez Peck <marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko <siguctua@gmail.com> wrote:
On 10 May 2011 09:48, Mariano Martinez Peck <marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker <marcus.denker@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 must be wrong, because the dev image when it is build, it calls all cleanUp methods for classes, so CompiledMethod class >> cleanUp shuld have been called, but if I take the Pharo1.3 result, I can see the source code (not decompiled)
But you can't see the source of those methods in browser, because browser works only with installed methods while cleanup touching only non-installed ones.
When i do this, my image hangs.
I have just tried and it crash!!! Actually, I evaluated CompiledMethod cleanUp. And just after cmd+b in CompiledMethod (to browse it) -> crash How it is possilbe it didn't crash while building the dev image?  or the first time I browse a class? maybe this incorrect CM are created by the build process just after the cleanUp ?
I don't know yet. Maybe there is some test, which breaking them when it runs? Or tests are not run on those images?
Now i thinking that it should actually use empty trailer for those methods i.e. CompiledMethodTrailer empty..
Still i found it strange why some methods has broken bytecode.
Yes!! and why they are not GCed!!
My theory is that some nasty code somewhere copied a compiled methods using some unsafe technique, like:
1 to: oldMethod size do: [ :i | newMethod at: i put: (oldMethod at: i) ]
I will search to see if I find something. The thing is that it is reproducible. Each new dev image build from scratch in a core, has these guys.
Because when you constructing compiled method normally (by compiling them) , there is no way how you can get broken compiled methods. (we would discover it much much earlier,
yes, exactly.
and in fact CompiledMethodTrailers are working for more than a year in both pharo and squeak, and there was no any issues like this from their side).
+1
-- Best regards, Igor Stasenko AKA sig.
-- Mariano http://marianopeck.wordpress.com
-- Best regards, Igor Stasenko AKA sig.
I must be wrong, because the dev image when it is build, it calls all cleanUp methods for classes, so CompiledMethod class >> cleanUp shuld
have
been called, but if I take the Pharo1.3 result, I can see the source code (not decompiled)
But you can't see the source of those methods in browser, because browser works only with installed methods while cleanup touching only non-installed ones.
grrrr sorry, I forgot about the #isInstalled ... I went directly to see #destroySourcePointer heheheh
When i do this, my image hangs.
I have just tried and it crash!!! Actually, I evaluated CompiledMethod cleanUp. And just after cmd+b in CompiledMethod (to browse it) -> crash How it is possilbe it didn't crash while building the dev image? or the first time I browse a class? maybe this incorrect CM are created by the build process just after the cleanUp ?
I don't know yet. Maybe there is some test, which breaking them when it runs? Or tests are not run on those images?
I was thinking exactly the same. But no, this image has not run the tests. There are other images specially to download from hudson that are those after running tests :( -- Mariano http://marianopeck.wordpress.com
The fix in http://code.google.com/p/pharo/issues/detail?id=4186 now one thing left: we need to get rid of hanging methods with corrupted trailers. And we do it like this: (CompiledMethod allInstances select: [:m | m isInstalled not and: [ m trailer kind = #VarLengthSourcePointer ]] ) do: [:each | each at: each size put: 0 ] This will replace a corrupted trailer with empty one. -- Best regards, Igor Stasenko AKA sig.
On Tue, May 10, 2011 at 1:44 PM, Igor Stasenko <siguctua@gmail.com> wrote:
The fix in http://code.google.com/p/pharo/issues/detail?id=4186
now one thing left: we need to get rid of hanging methods with corrupted trailers.
And we do it like this:
(CompiledMethod allInstances select: [:m | m isInstalled not and: [ m trailer kind = #VarLengthSourcePointer ]] ) do: [:each | each at: each size put: 0 ]
This will replace a corrupted trailer with empty one.
Excellent!!! that worked :) Thanks Igor. Now...it is supposed that when this issue fix is integrated the new generated image should now have these weird CMs so that script should not be necessary. I will test it when such image is build by hudson :) -- Mariano http://marianopeck.wordpress.com
On 10.05.2011 12:44, Igor Stasenko wrote:
On 10 May 2011 11:27, Mariano Martinez Peck<marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko<siguctua@gmail.com> wrote:
On 10 May 2011 09:48, Mariano Martinez Peck<marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker<marcus.denker@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 ;) Cheers, Henry
On 10 May 2011 13:33, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 10.05.2011 12:44, Igor Stasenko wrote:
On 10 May 2011 11:27, Mariano Martinez Peck<marianopeck@gmail.com> Â wrote:
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko<siguctua@gmail.com> Â wrote:
On 10 May 2011 09:48, Mariano Martinez Peck<marianopeck@gmail.com> Â wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker<marcus.denker@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. 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?
Cheers, Henry
-- Best regards, Igor Stasenko AKA sig.
On 10 May 2011 16:22, Igor Stasenko <siguctua@gmail.com> wrote:
On 10 May 2011 13:33, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 10.05.2011 12:44, Igor Stasenko wrote:
On 10 May 2011 11:27, Mariano Martinez Peck<marianopeck@gmail.com> Â wrote:
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko<siguctua@gmail.com> Â wrote:
On 10 May 2011 09:48, Mariano Martinez Peck<marianopeck@gmail.com> Â wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker<marcus.denker@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.
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?
oh , and forgot to note, that having a source code available at all times is quite useful in cases, when your methods using some kind of DSL. Because decompiler can only reconstruct a source code of smalltalk methods, but not the ones, which were authored using customized parser/compiler .. like using Helvetia.
Cheers, Henry
-- Best regards, Igor Stasenko AKA sig.
-- Best regards, Igor Stasenko AKA sig.
oh , and forgot to note, that having a source code available at all times is quite useful in cases, when your methods using some kind of DSL. Because decompiler can only reconstruct a source code of smalltalk methods, but not the ones, which were authored using customized parser/compiler .. like using Helvetia.
Btw, the change i proposing is _minimal_ and straightforward: basicAddSelector: selector withMethod: compiledMethod "Add the message selector with the corresponding compiled method to the receiver's method dictionary. Do this without sending system change notifications" | oldMethodOrNil | oldMethodOrNil := self lookupSelector: selector. self methodDict at: selector put: compiledMethod. compiledMethod methodClass: self. compiledMethod selector: selector. "Now flush Pharo's method cache, either by selector or by method" oldMethodOrNil ifNotNil: [ oldMethodOrNil flushCache. ++++ oldMethodOrNil dropSourcePointer. ]. selector flushCache. :) -- Best regards, Igor Stasenko AKA sig.
here an example how it works. first evaluate TTT blockValue so it assigns Block with closure returned by methodWithBlock. Now go to #methodWithBlock and change its implementation. And then try to debug TTT blockValue and you can see that when you evaluating a block , it will show you an original source code of method. (Except that there is some bug in debugger, which erasing the text area once you click on frame with block. But when you click it second time it show it normally). On 10 May 2011 16:46, Igor Stasenko <siguctua@gmail.com> wrote:
oh , and forgot to note, that having a source code available at all times is quite useful in cases, when your methods using some kind of DSL. Because decompiler can only reconstruct a source code of smalltalk methods, but not the ones, which were authored using customized parser/compiler .. like using Helvetia.
Btw, the change i proposing is _minimal_ and straightforward:
basicAddSelector: selector withMethod: compiledMethod     "Add the message selector with the corresponding compiled method to the     receiver's method dictionary.     Do this without sending system change notifications"
    | oldMethodOrNil |     oldMethodOrNil := self lookupSelector: selector.     self methodDict at: selector put: compiledMethod.     compiledMethod methodClass: self.     compiledMethod selector: selector.
    "Now flush Pharo's method cache, either by selector or by method"     oldMethodOrNil ifNotNil: [         oldMethodOrNil flushCache. ++++   oldMethodOrNil dropSourcePointer.         ].     selector flushCache.
:)
-- Best regards, Igor Stasenko AKA sig.
-- Best regards, Igor Stasenko AKA sig.
Since code.google.com is broken, i putting it here. Embed source code for methods which get uninstalled from their classes. When compiled method is deinstalled from its class, its not managed anymore by development tools and it's hard to predict, how long a method could stay in the image, because if it contains blocks,they could still reference it. This change makes sure that when you deinstall method from its owning class, its source code are preserved in an embedded form inside a trailer. This is in addition to issue #... that one.. cant see it without google working, sorry :) -- Best regards, Igor Stasenko AKA sig.
On 10.05.2011 16:22, Igor Stasenko wrote:
On 10 May 2011 13:33, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 10.05.2011 12:44, Igor Stasenko wrote:
On 10 May 2011 11:27, Mariano Martinez Peck<marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko<siguctua@gmail.com> wrote:
On 10 May 2011 09:48, Mariano Martinez Peck<marianopeck@gmail.com> wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker<marcus.denker@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
On 12 May 2011 16:02, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:
On 10.05.2011 16:22, Igor Stasenko wrote:
On 10 May 2011 13:33, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> Â wrote:
On 10.05.2011 12:44, Igor Stasenko wrote:
On 10 May 2011 11:27, Mariano Martinez Peck<marianopeck@gmail.com> Â wrote:
On Tue, May 10, 2011 at 10:59 AM, Igor Stasenko<siguctua@gmail.com> Â wrote:
On 10 May 2011 09:48, Mariano Martinez Peck<marianopeck@gmail.com> Â wrote:
On Tue, May 10, 2011 at 9:41 AM, Marcus Denker<marcus.denker@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.
There is #destroySourcePointer and #dropSourcePointer. First just destroys it, and don't attempts to retrieve the source code , because as you mentioned it may be not valid anymore. While #dropSourcePointer replacing a source pointer with embedded source code, and of course you should do it before condensing .changes or .sources, when source pointers are still pointing to correct place(s). CompiledMethod>>cleanUp using #destroySourcePointer, so there should be no problem with that.
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.
It seems like that with this change i opened a can of worms. There are some tricky logic somewhere ,which uninstalls methods from classes and then adds them back. So, i added #dropSourcePointer at uninstalling .. then compilation becomes too slow (because embedding source code using #becomeForward: ). Then i changed #dropSourcePointer to actually erase the trailer without using #become .. but then some methods lost their source.
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
-- Best regards, Igor Stasenko AKA sig.
Igor could you open a ticket that we do not forget. Stef
It seems like that with this change i opened a can of worms. There are some tricky logic somewhere ,which uninstalls methods from classes and then adds them back. So, i added #dropSourcePointer at uninstalling .. then compilation becomes too slow (because embedding source code using #becomeForward: ). Then i changed #dropSourcePointer to actually erase the trailer without using #become .. but then some methods lost their source.
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
-- Best regards, Igor Stasenko AKA sig.
On 12 May 2011 20:15, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Igor
could you open a ticket that we do not forget.
there is already one,where we discussing that http://code.google.com/p/pharo/issues/detail?id=4200
Stef
It seems like that with this change i  opened a can of worms. There are some tricky logic somewhere ,which uninstalls methods from classes and then adds them back. So, i added #dropSourcePointer at uninstalling .. then compilation becomes too slow (because embedding source code using #becomeForward: ). Then i changed #dropSourcePointer to actually erase the trailer without using #become .. but then some methods lost their source.
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
-- Best regards, Igor Stasenko AKA sig.
-- Best regards, Igor Stasenko AKA sig.
participants (5)
-
Henrik Sperre Johansen -
Igor Stasenko -
Marcus Denker -
Mariano Martinez Peck -
Stéphane Ducasse