Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Status: FixReviewNeeded
Owner: [hidden email]
CC: [hidden email]
Labels: Milestone-1.4

New issue 5276 by [hidden email]: replaced MCMethodDefinition's  
Definitions class variable with a class instance variable.
http://code.google.com/p/pharo/issues/detail?id=5276

levente did the following in squeak

- replaced MCMethodDefinition's Definitions class variable with a class  
instance variable. The cached definitions are no longer registered for  
finalization.
- a bit of cleanup around MCDefinition's Instances class variable


MCdefinition class>>instanceLike: aDefinition

        ^(instances ifNil: [ instances := WeakSet new ])
                like: aDefinition
                ifAbsent: [ instances add: aDefinition ]

cachedDefinitions
        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.  
WeakArray addWeakDependent: Definitions].
        ^ Definitions

=>

MCMethodDefinition>>cachedDefinitions
       
        ^definitions ifNil: [ definitions := WeakIdentityKeyDictionary new ]



igor I want to have look at that with you.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo

Comment #1 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

Object subclass: #MCDefinition
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Monticello-Base'

MCDefinition class
        instanceVariableNames: 'instances'

MCDefinition class>>clearInstances

        instances := nil.
        self subclassesDo: #clearInstances




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo

Comment #2 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

This was:

[hidden email] via lists.squeakfoundation.org
       
28/02/10
               
à squeak-dev, packages
Levente Uzonyi uploaded a new version of Monticello to project The Trunk:
http://source.squeak.org/trunk/Monticello-ul.375.mcz

==================== Summary ====================

Name: Monticello-ul.375
Author: ul
Time: 28 February 2010, 2:58:21.945 pm
UUID: 2e3b9ffd-b728-364b-84c2-1f189c89fcdf
Ancestors: Monticello-ul.374

- replaced MCDefinition's Instances class variable with a class instance  
variable. Smaller WeakSets are better than a large one.

=============== Diff against Monticello-ul.374 ===============

Item was changed:
  Object subclass: #MCDefinition
        instanceVariableNames: ''
+       classVariableNames: ''
-       classVariableNames: 'Instances'
        poolDictionaries: ''
        category: 'Monticello-Base'!
+ MCDefinition class
+       instanceVariableNames: 'instances'!
+ MCDefinition class
+       instanceVariableNames: 'instances'!

Item was changed:
  ----- Method: MCDefinition class>>instanceLike: (in category 'as yet  
unclassified') -----
  instanceLike: aDefinition

+       ^(instances ifNil: [ instances := WeakSet new ])
-       ^(Instances ifNil: [ Instances := WeakSet new ])
                like: aDefinition
+               ifAbsent: [ instances add: aDefinition ]!
-               ifAbsent: [ Instances add: aDefinition ]!

Item was changed:
+ (PackageInfo named: 'Monticello') preamble: ''!
- (PackageInfo named: 'Monticello') preamble: 'WeakArray removeDependent:  
(MCMethodDefinition classPool at: #Definitions)'!

Item was changed:
  ----- Method: MCDefinition class>>clearInstances (in category 'as yet  
unclassified') -----
  clearInstances

+       instances := nil.
+       self subclassesDo: #clearInstances!
-       Instances := nil!

_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo

Comment #3 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

And

Levente Uzonyi uploaded a new version of Monticello to project The Trunk:
http://source.squeak.org/trunk/Monticello-ul.374.mcz

==================== Summary ====================

Name: Monticello-ul.374
Author: ul
Time: 28 February 2010, 2:57:06.129 pm
UUID: c8779e3f-6e45-2d4a-828e-d6e02cb40546
Ancestors: Monticello-ar.372

- replaced MCMethodDefinition's Definitions class variable with a class  
instance variable. The cached definitions are no longer registered for  
finalization.
- a bit of cleanup around MCDefinition's Instances class variable

=============== Diff against Monticello-ar.372 ===============

Item was changed:
  ----- Method: MCMethodDefinition class>>shutDown (in category 'as yet  
unclassified') -----
  shutDown
+
+       definitions := nil.!
-       WeakArray removeWeakDependent: Definitions.
-       Definitions := nil.!

Item was changed:
  ----- Method: MCMethodDefinition class>>cachedDefinitions (in category 'as  
yet unclassified') -----
  cachedDefinitions
+
+       ^definitions ifNil: [ definitions := WeakIdentityKeyDictionary new  
]!
-       Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.  
WeakArray addWeakDependent: Definitions].
-       ^ Definitions!

Item was changed:
  MCDefinition subclass: #MCMethodDefinition
        instanceVariableNames: 'classIsMeta source category selector  
className timeStamp'
+       classVariableNames: ''
-       classVariableNames: 'Definitions'
        poolDictionaries: ''
        category: 'Monticello-Modeling'!
+ MCMethodDefinition class
+       instanceVariableNames: 'definitions'!
+ MCMethodDefinition class
+       instanceVariableNames: 'definitions'!

Item was changed:
  ----- Method: MCDefinition class>>instanceLike: (in category 'as yet  
unclassified') -----
  instanceLike: aDefinition
+
+       ^(Instances ifNil: [ Instances := WeakSet new ])
+               like: aDefinition
+               ifAbsent: [ Instances add: aDefinition ]!
-       Instances ifNil: [Instances := WeakSet new].
-       ^ (Instances like: aDefinition) ifNil: [Instances add: aDefinition]!

Item was added:
+ (PackageInfo named: 'Monticello') preamble: 'WeakArray removeDependent:  
(MCMethodDefinition classPool at: #Definitions)'!

Item was changed:
  ----- Method: MCDefinition class>>clearInstances (in category 'as yet  
unclassified') -----
  clearInstances
+
-       WeakArray removeWeakDependent: Instances.
        Instances := nil!

Item was removed:
- ----- Method: MCSnapshotBrowser>>aboutToStyle: (in category 'morphic ui')  
-----
- aboutToStyle: aStyler
-
-       | classOrMetaClass shouldStyle |
-       classSelection ifNil: [ ^false ].
-       switch = #comment ifTrue: [ ^false ].
-       (protocolSelection notNil and: [
-               methodSelection notNil ])
-                       ifTrue: [
-                               classOrMetaClass := self  
selectedClassOrMetaClass.
-                               shouldStyle := true ]
-                       ifFalse: [
-                               classOrMetaClass := nil.
-                               shouldStyle := categorySelection ~= self  
extensionsCategory ].
-       aStyler classOrMetaClass: classOrMetaClass.
-       ^shouldStyle!


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Status: Workneeded
        Labels: Type-Enh

Comment #4 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Labels: -Milestone-1.4

Comment #5 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

Not a show stopper for releasing 1.4. (but *if* someone posts code, we are  
happy to consider it to 1.4.. but nobody needs to stay awake and fix it  
before we can release 1.4)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Labels: Milestone-2.0

Comment #6 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Status: WorkNeeded

Comment #7 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Labels: -Milestone-2.0

Comment #8 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

Not a show stopper for 2.0


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo

Comment #9 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

Igor this is related to the bug we fixed on MC so I would like to have a  
look at it with you. I will prepare a slice.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo

Comment #10 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

We turned the classVar usage into instance variables and we clean the like:  
use which does not work on dictionary. Now we are not sure that this is  
correct.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Status: FixReviewNeeded
        Cc: [hidden email]

Comment #11 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

camillo can you have a look?
because this is related to the method cache.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo

Comment #12 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

I like it :), for each class variable 10 kitties and a butterfly die, so I  
am glad we invest in our fauna...


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Labels: Milestone-2.0

Comment #13 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Status: MonkeyIsChecking

Comment #14 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276#c14

The Monkey is currently checking this issue. Please don't change it!


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Status: WorkNeeded

Comment #15 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276#c15

Monkey went bananas:
--------------------
Error while loading  
SLICE-Issue-5276-replaced-MCMethodDefinitions-Definitions-class-variable-with-a-class-instance-variable-StephaneDucasse.1  
from http://ss3.gemstone.com/ss/PharoInbox:
        UndeclaredVariableWarning: MCDefinition class>>#clearInstances  
('Instances' is Undeclared)
  1: EncoderForV3PlusClosures(Encoder)>>undeclared:
  2: Parser>>correctVariable:interval:
  3: [self
                        correctVariable: varName
                        interval: (varStart to: varEnd)] in Parser>>variable
  4: [(self
                                lookupInPools: name
                                ifFound: [:assoc | varNode := self global: assoc name: name])
                        ifTrue: [varNode]
                        ifFalse: [^ action value]] in  
EncoderForV3PlusClosures(Encoder)>>encodeVariable:sourceRange:ifUnknown:
  5: Dictionary>>at:ifAbsent:
  6: EncoderForV3PlusClosures(Encoder)>>encodeVariable:sourceRange:ifUnknown:
  7: Parser>>variable
  8: Parser>>primaryExpression
  9: Parser>>messagePart:repeat:
10: Parser>>expression
        ...
----------------------------------------------------------
Loaded Source:  
SLICE-Issue-5276-replaced-MCMethodDefinitions-Definitions-class-variable-with-a-class-instance-variable-StephaneDucasse.1  
from http://ss3.gemstone.com/ss/PharoInbox
Tested using Pharo-2.0-20493-a on CoInterpreter  
VMMaker-oscog-EstebanLorenzano.166 uuid:  
5773fcb9-2982-4507-8a9e-4308ec33731e Dec 12 2012
StackToRegisterMappingCogit VMMaker-oscog-EstebanLorenzano.166 uuid:  
5773fcb9-2982-4507-8a9e-4308ec33731e Dec 12 2012
git://gitorious.org/cogvm/blessed.git Commit:  
452863bdfba2ba0b188e7b172e9bc597a2caa928 Date: 2012-12-07 16:49:46 +0100  
By: Esteban Lorenzano <[hidden email]> Jenkins build #5922


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5276 in pharo: replaced MCMethodDefinition's Definitions class variable with a class instance variable.

pharo
Updates:
        Labels: -Milestone-2.0 Milestone-3.0

Comment #16 on issue 5276 by [hidden email]: replaced  
MCMethodDefinition's Definitions class variable with a class instance  
variable.
http://code.google.com/p/pharo/issues/detail?id=5276

I think we should stop changing monticello in 2.0 other then for fixing  
bugs. I am starting to get tired... we will add this, it will have some  
grave grave side-effect nobody was expecting --> 2.0 delays another week or  
so.

Let's move it to 3.0


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