Re: Issue 5005 in pharo: #cleanUpForRelease is broken: MessageNotUnderstood: ByteSymbol>>run:with:in:

Updates:
        Summary: #cleanUpForRelease  is broken: MessageNotUnderstood:  
ByteSymbol>>run:with:in:

Comment #2 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Labels: Milestone-1.4 Type-Bug

Comment #3 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #4 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Well... I have just confirmed in issue  
http://code.google.com/p/pharo/issues/detail?id=5116   that this is not  
related to rehash since even comment such line it still fails.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

I discovered it is relatedt to the Smalltalk cleanUp. Moreoever, I can  
reproduce it with this code:

Smalltalk cleanUp: true except: #() confirming: false.
        Smalltalk
                allClassesAndTraitsDo: [ :class |
                        [ :each |
                                each
                                        removeEmptyCategories;
                                        sortCategories ]
                                                value: class organization;
                                                value: class class organization ].


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #6 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

So...I was removing the rehash of ScriptLoader but it seems it is called  
#compactAll because

MethodDictionary >> cleanUp: aggressive
        "Rehash all instances when cleaning aggressively"

        aggressive ifTrue: [self compactAll].


So the problem may not be in the #rehash but in the #compactAllInstances


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: -[hidden email]

Comment #7 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

no...it is not the #compactAll .. some other #cleanUp is guilty


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Ok...we are getting close. To reproduce:

ClassOrganizer cleanUp: true.
        Smalltalk
                allClassesAndTraitsDo: [ :class |
                        [ :each |
                                each
                                        removeEmptyCategories;
                                        sortCategories ]
                                                value: class organization;
                                                value: class class organization ].


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Blockedon: 5192

Comment #9 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Mariano discovered methodDictionary could get improper size due to sizeFor:  
using asLargerPowerOfTwo, which returns 0 for 0 argument, due to bug in  
isPowerOfTwo.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Blockedon: -5192

Comment #10 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Unblocked, the changes to largerPower* methods will probably lead to  
DomainErrors for
0 asLargerPowerOfTwo due to implementing protocol on Fraction as well.

Guard should be in a different place than in Squeak though:

sizeFor: numberOfElements
     "Return the minimum capacity of a dictionary that can hold  
numberOfElements elements. At least 25% of the array must be empty and the  
return value must be a nonnegative power of 2."

     ^(numberOfElements * 4 // 3 max: 1) asLargerPowerOfTwo

This fix is fully orthagonal to fixing bugs/inconsistensies in sending 0  
isPowerOfTwo/asLargerPowerOfTwo




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #11 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Hi Henry, Indeed, that version if sizeFor: solves the problem and it is  
independent of the issue 5192. So I think we can integrate this version.
Maybe adding a nice comment to explain the max:1  ?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #12 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

"the max: 1 is because even empty MethodDictionaries should have one  
slot"  ?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #13 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

After 5192, if you try to remove the guard and reshape to fit 0 elements,  
you will get a DomainError raised from  largerPowerOfTwo with a message  
text that a larger power of two is only defined in the interval (0 ,  
infinity), shouldn't that be enough? ;)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #14 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

I have no idea how rehashing ends up creating empty ones, but a non-empty  
original instance seems to be an invariant for at least #grow to work  
correctly. (2 * 0 is still 0)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #15 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

oh, compactWithoutBecome potentially does that...
What silly person would write that, and assume new: actually gives you a  
valid instance when size is zero without checking! :)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #16 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Ok guys let me know.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #17 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Hi Henry. I have to admit I am a little bit lost. I still don't understand  
exactly what was the problem. I do understand that #sizeFor:  would answer  
0 when the argument is 0 as well. But what I don't understand is how that  
can affect #compactWithoutBecome. how? in which scenarios?  why it doesn't  
affect #grow?
Sorry for the questions but I want to understand.

Thanks!


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #18 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Moroever, I don't understan why Levente says " even empty  
MethodDictionaries should have one slot"  why ?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #19 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

The other way around, compactWithoutBecome  affects sizeFor: , by calling  
it with 0 in the case where it compacts instances which had had their  
entire contents removed since the last compaction.

Which leads to bad things when someone tries to insert new methods in this  
dictionary, as grow will fail to actually grow (The implementation assumes  
size > 0)
Which is also the reason even empty MethodDictionaries should have one  
slot, so they won't fail if someone tries to add to them.
(This is also the comment you should put in sizeFor:; a MethodDictionaries  
can never be entirely empty, as the #grow method requires it not to be)

As for exactly why/how that manifests in the way described earlier in this  
issue, I have no clue.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixToInclude
        Cc: [hidden email]

Comment #20 on issue 5005 by marianopeck: #cleanUpForRelease  is broken:  
MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

Thanks Henry. Now I understand :)
I even understand what previously it was working:

new: nElements
        "Create a Dictionary large enough to hold nElements without growing.
        Note that the basic size must be a power of 2.
        It is VITAL (see grow) that size gets doubled if nElements is a power of 2"
       
        | size |
        size := 1 bitShift: nElements highBit.
        ^ (self basicNew: size) initialize: size
       

and  ->  1 bitShift: 0 highBit.  -> 1  :)

Thanke Henry. Fix is in inbox:

Name:  
SLICE-Issue-5005-cleanUpForRelease-is-broken-MessageNotUnderstood-ByteSymbolrunwithin-MarianoMartinezPeck.1
Author: MarianoMartinezPeck
Time: 18 January 2012, 7:17:47 pm
UUID: 41cd5c6f-831a-45d7-aadd-b9d402f13b8e
Ancestors:
Dependencies: Kernel-MarianoMartinezPeck.996

fix to issue 5005


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Closed

Comment #21 on issue 5005 by [hidden email]: #cleanUpForRelease  is  
broken: MessageNotUnderstood: ByteSymbol>>run:with:in:
http://code.google.com/p/pharo/issues/detail?id=5005

in 14287


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker