[Pharo-project] (igor) Help for 1.1: condenseChanges does not work anymore
Hi We cannot do Smalltalk condenseChanges and this is a showStopper for 1.1 beta. http://code.google.com/p/pharo/issues/detail?id=2367 After some hours of debugging, Lukas has the intuition that this is related to methodTrailer. I did some stupid tests... and apparently the problem occured between 11117 and 11128 If you take 11117 and run Utilities readServerUpdatesThrough: 3922 saveLocally: true updateImage: true and in a 11120 image Smalltalk condenseChanges is working. So the problem is somehow related to methodTrailer. Igor can you have a look? Stef 11118 - Issue 1701: ArchiveViewer now works on Pharo - Issue 1679: InputEventSensor refers to defunct class InputSensor - Issue 1703: Closing last windows raise error - Issue 1698: Enhancement: trunk #collect:as: is a must 11119 - Issue 1711: Cleaning host menu' 11120 - Cleaning some tests. #testSort, #testRestoreDefaultFonts - Issue 1691: Adding some tests for formalParameterAt: from nicolas - Moved FixedIdentitySet to Collections-Unordered - Better comments. - Better shuffledBy: - Issue 1713: Clean Duplication in widthOfString:from:to: 11121 - Issue 1690: New Method Trailer part 1 (cs 1_ and 2_) 11122 - Issue 1690: New Method Trailer part 2 (cs 3 merged) 11123 - Issue 1690: New Method Trailer part 3 (cs 4 no conflict) 11124 - Issue 1690: New Method Trailer part 4 (cs 5) 11125 - Issue 1690: New Method Trailer part 5 (cs 6 + postscript cs7) 11126 - Issue 1690: New Method Trailer part 6 (cs 8 - trailer) 11127 - Issue 1690: New Method Trailer part 7 (cs 9) 11128 - Issue 1690: New Method Trailer post clean up :)
Hi. I made a little investigation.. The changes file condensing works fine. Except that there something wrong with destroying source pointers of not-installed methods. After commenting a following line: " CompiledMethod allInstancesDo: [:e | e isInstalled ifFalse: [e destroySourcePointer]]. " It were able to run to an end, and by running this check: | empty | empty := Set new. Smalltalk allClassesAndTraitsDo: [:classOrTrait | empty addAll: ( classOrTrait methodDict select: [:m | m sourcePointer =0 ]) ]. empty isEmpty -->> prints true which means there is no installed methods with sourcePointer = 0 (and hence with no source code) after condensing the changes. But what makes me really suspicious is following: CompiledMethod allInstances size 55797 (CompiledMethod allInstances reject: #isInstalled) size 12586 it looks like #isInstalled is buggy, or otherwise indicating that there is problem with image, which keeps so many non-installed methods hanging around. I would simply put a following at the beginning of #condenseChanges: (CompiledMethod allInstances reject: #isInstalled) size > 0 ifTrue: [ self error: 'first, get rid of any uninstalled methods' ]. So, then you would not need to use #destroySourcePointer :) Here an alternative version of #destroySourcePointer. But it doesn't makes any big difference. destroySourcePointer "We can't change the trailer of existing method, since it could have completely different format. Therefore we need to generate a copy with new trailer, and then #become it" | copy | copy := self copyWithTrailerBytes: CompiledMethodTrailer empty. self becomeForward: copy. ^ copy P.S. runned it on 1.1-11343-UNSTABLE -- Best regards, Igor Stasenko AKA sig.
Here's another calculation: | size | size := 0. Smalltalk allClassesAndTraitsDo: [:classOrTrait | size := size + classOrTrait methodDict size ]. size 37516 so, it looks like there really about 12k methods hanging around and not installed anywhere .. :) -- Best regards, Igor Stasenko AKA sig.
Oh, and i think there's no problem with #destroySourcePointer it just takes too long, and looks like your image is hung, because you can't wait untill it chews all 12k methods.. Just tried to do it with a random method: CompiledMethod allInstances atRandom copy destroySourcePointer getSource and it works fine. -- Best regards, Igor Stasenko AKA sig.
And the last thing... since new trailers using #becomeForward: no wonder that this loop turns into an infinite: CompiledMethod allInstancesDo: [:e | e isInstalled ifFalse: [e destroySourcePointer]]. because #allInstancesDo: walks a heap, and assuming that there is no new instances of it created during walking. But destroySourcePointer creating a new instance, so it can never finish! :) i just replaced it with following: (CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]]. and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;) -- Best regards, Igor Stasenko AKA sig.
After condensing changes and #destroySourcePointer, the garbage 12k methods magically disappeared, so now its just: (CompiledMethod allInstances reject: #isInstalled) size 67 and image after saving shrunk from 14M to 13M probably some cache somewhere not flushed.. -- Best regards, Igor Stasenko AKA sig.
probably flushing the cache of MC and PragmaBuilder Stef On May 9, 2010, at 4:02 AM, Igor Stasenko wrote:
After condensing changes and #destroySourcePointer, the garbage 12k methods magically disappeared, so now its just:
(CompiledMethod allInstances reject: #isInstalled) size 67
and image after saving shrunk from 14M to 13M probably some cache somewhere not flushed..
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On 9 May 2010 12:06, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
probably flushing the cache of MC and PragmaBuilder
PragmaBuilder seems to refer to a couple of compiled methods that are not installed in the system. Lukas -- Lukas Renggli www.lukas-renggli.ch
Stef On May 9, 2010, at 3:47 AM, Igor Stasenko wrote:
And the last thing... since new trailers using #becomeForward: no wonder that this loop turns into an infinite:
CompiledMethod allInstancesDo: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
because #allInstancesDo: walks a heap, and assuming that there is no new instances of it created during walking. But destroySourcePointer creating a new instance, so it can never finish! :)
i just replaced it with following:
(CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
Yes clearly a conceptual bug. But with the solution alain proposed ie not destroying while iterating on methods does not work (yes it condenses well but accessing some methods is broken). Lukas did a cool test that checks whether the we can parse the method.
and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;)
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Hi all,
i just replaced it with following:
(CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
Yes clearly a conceptual bug. But with the solution alain proposed ie not destroying while iterating on methods does not work (yes it condenses well but accessing some methods is broken).
yes but Igor solution is a little bit different because 'e isInstalled' is tested twice, one for the reject: and the second in the do: block. But unfortunately, this solution seems to be broken to. It works one time, but while trying to condenseChanges again several time, my squeak process is killed without flushing any log. Alain
Lukas did a cool test that checks whether the we can parse the method.
and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;)
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
but do you save and quit the image. I added a fix to avoid iterate while changing the collection and we will see. On May 9, 2010, at 12:29 PM, Alain Plantec wrote:
Hi all,
i just replaced it with following:
(CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
Yes clearly a conceptual bug. But with the solution alain proposed ie not destroying while iterating on methods does not work (yes it condenses well but accessing some methods is broken).
yes but Igor solution is a little bit different because 'e isInstalled' is tested twice, one for the reject: and the second in the do: block. But unfortunately, this solution seems to be broken to. It works one time, but while trying to condenseChanges again several time, my squeak process is killed without flushing any log. Alain
Lukas did a cool test that checks whether the we can parse the method.
and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;)
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Alain with the latest unstable it seems to work. You just have to wait a lot once the progress bar is finished to display. Then we got only 34/35 methods not installed which is ok. I will fix condenseSources then :) Stef
Arghhhhhhh Apparently I did two condenseChanges and I get a problem with code. I could not browse anymore condenseChanges On May 9, 2010, at 1:09 PM, Stéphane Ducasse wrote:
Alain with the latest unstable it seems to work. You just have to wait a lot once the progress bar is finished to display.
Then we got only 34/35 methods not installed which is ok. I will fix condenseSources then :) Stef
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On 9 May 2010 13:29, Alain Plantec <alain.plantec@free.fr> wrote:
Hi all,
i just replaced it with following:
(CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
Yes clearly a conceptual bug. But with the solution alain proposed ie not destroying while iterating on methods does not work (yes it condenses well but accessing some methods is broken).
yes but Igor solution is a little bit different because 'e isInstalled' is tested twice, one for the reject: and the second in the do: block.
This was a result of lack of attention, not my real intent. :) In bug entry for this issue i added a method which does just: (CompiledMethod allInstances reject: #isInstalled) do: #destroySourcePointer. there's no way how method can become installed when you changin its trailer. So, a single test #isInstalled is sufficient. A more faster way is to use a mass-become. But i think this is not a place where you need to care too much about performance.
But unfortunately, this solution seems to be broken to. It works one time, but while trying to condenseChanges again several time, my squeak process is killed without flushing any log.
You mean running #condenseChanges multiple times in a row, or interrupting it and then start over again? You should know, that it is important to not interrupt it in the middle, because when you doing that, part of your methods having old source pointers, while rest part having new ones, and there is no way to tell, which one is correct.
Alain
Lukas did a cool test that checks whether the we can parse the method.
and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;)
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Best regards, Igor Stasenko AKA sig.
Probably , a more crash-proof #condenseChanges would be to use a mass-become. So, you are not modifying existing methods, but making their copies with updated source pointers and copying their source into new changes file. Then you do a single mass-become and so, all old methods get swapped with new ones using a single massive blow. In that way, its more safe, since if you interrupt the #condenseChanges in the middle, you can use debugger and browse the code , and if you decide to abort it, nothing bad happen, since methods with new source pointers is held in your temp collection and not yet applied. On 9 May 2010 19:48, Igor Stasenko <siguctua@gmail.com> wrote:
On 9 May 2010 13:29, Alain Plantec <alain.plantec@free.fr> wrote:
Hi all,
i just replaced it with following:
(CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
Yes clearly a conceptual bug. But with the solution alain proposed ie not destroying while iterating on methods does not work (yes it condenses well but accessing some methods is broken).
yes but Igor solution is a little bit different because 'e isInstalled' is tested twice, one for the reject: and the second in the do: block.
This was a result of lack of attention, not my real intent. :)
In bug entry for this issue i added a method which does just: (CompiledMethod allInstances reject: #isInstalled) do: #destroySourcePointer.
there's no way how method can become installed when you changin its trailer. So, a single test #isInstalled is sufficient. A more faster way is to use a mass-become. But i think this is not a place where you need to care too much about performance.
But unfortunately, this solution seems to be broken to. It works one time, but while trying to condenseChanges again several time, my squeak process is killed without flushing any log.
You mean running #condenseChanges multiple times in a row, or interrupting it and then start over again? You should know, that it is important to not interrupt it in the middle, because when you doing that, part of your methods having old source pointers, while rest part having new ones, and there is no way to tell, which one is correct.
Alain
Lukas did a cool test that checks whether the we can parse the method.
and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;)
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Best regards, Igor Stasenko AKA sig.
-- Best regards, Igor Stasenko AKA sig.
The problem was mainly that the UI did not show that it was destroying a lot of methods and I thought it was blocked. So if you wait enough it seems ok. Lukas checked and reparse and check the system and it seems ok. Stef On May 9, 2010, at 6:55 PM, Igor Stasenko wrote:
Probably , a more crash-proof #condenseChanges would be to use a mass-become. So, you are not modifying existing methods, but making their copies with updated source pointers and copying their source into new changes file. Then you do a single mass-become and so, all old methods get swapped with new ones using a single massive blow. In that way, its more safe, since if you interrupt the #condenseChanges in the middle, you can use debugger and browse the code , and if you decide to abort it, nothing bad happen, since methods with new source pointers is held in your temp collection and not yet applied.
On 9 May 2010 19:48, Igor Stasenko <siguctua@gmail.com> wrote:
On 9 May 2010 13:29, Alain Plantec <alain.plantec@free.fr> wrote:
Hi all,
i just replaced it with following:
(CompiledMethod allInstances reject: #isInstalled) do: [:e | e isInstalled ifFalse: [e destroySourcePointer]].
Yes clearly a conceptual bug. But with the solution alain proposed ie not destroying while iterating on methods does not work (yes it condenses well but accessing some methods is broken).
yes but Igor solution is a little bit different because 'e isInstalled' is tested twice, one for the reject: and the second in the do: block.
This was a result of lack of attention, not my real intent. :)
In bug entry for this issue i added a method which does just: (CompiledMethod allInstances reject: #isInstalled) do: #destroySourcePointer.
there's no way how method can become installed when you changin its trailer. So, a single test #isInstalled is sufficient. A more faster way is to use a mass-become. But i think this is not a place where you need to care too much about performance.
But unfortunately, this solution seems to be broken to. It works one time, but while trying to condenseChanges again several time, my squeak process is killed without flushing any log.
You mean running #condenseChanges multiple times in a row, or interrupting it and then start over again? You should know, that it is important to not interrupt it in the middle, because when you doing that, part of your methods having old source pointers, while rest part having new ones, and there is no way to tell, which one is correct.
Alain
Lukas did a cool test that checks whether the we can parse the method.
and made it to the end.. took about 5 minutes to wait. (12k methods + become on every method is a little time consuming ;)
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Best regards, Igor Stasenko AKA sig.
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On 9 May 2010 20:03, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
The problem was mainly that the UI did not show that it was destroying a lot of methods and I thought it was blocked. So if you wait enough it seems ok. Lukas checked and reparse and check the system and it seems ok.
Yeah, i also had some troubles with interrupting it, caused by running an infinite loop. :)
Stef
On May 9, 2010, at 6:55 PM, Igor Stasenko wrote:
-- Best regards, Igor Stasenko AKA sig.
Hi. I made a little investigation..
Thanks.
The changes file condensing works fine.
Yes we got that too. And sometimes it did not finished depending on the actual changes. The problem was in that case that after some methods were impossible to browse.
Except that there something wrong with destroying source pointers of not-installed methods.
After commenting a following line: " CompiledMethod allInstancesDo: [:e | e isInstalled ifFalse: [e destroySourcePointer]]. "
It were able to run to an end, and by running this check:
| empty | empty := Set new. Smalltalk allClassesAndTraitsDo: [:classOrTrait | empty addAll: ( classOrTrait methodDict select: [:m | m sourcePointer =0 ]) ]. empty isEmpty -->> prints true
which means there is no installed methods with sourcePointer = 0 (and hence with no source code) after condensing the changes.
But what makes me really suspicious is following:
CompiledMethod allInstances size 55797 (CompiledMethod allInstances reject: #isInstalled) size 12586
it looks like #isInstalled is buggy, or otherwise indicating that there is problem with image, which keeps so many non-installed methods hanging around. I would simply put a following at the beginning of #condenseChanges:
(CompiledMethod allInstances reject: #isInstalled) size > 0 ifTrue: [ self error: 'first, get rid of any uninstalled methods' ].
So, then you would not need to use #destroySourcePointer :)
Here an alternative version of #destroySourcePointer. But it doesn't makes any big difference.
destroySourcePointer
"We can't change the trailer of existing method, since it could have completely different format. Therefore we need to generate a copy with new trailer, and then #become it" | copy | copy := self copyWithTrailerBytes: CompiledMethodTrailer empty. self becomeForward: copy. ^ copy
P.S. runned it on 1.1-11343-UNSTABLE
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
participants (4)
-
Alain Plantec -
Igor Stasenko -
Lukas Renggli -
Stéphane Ducasse