The Inbox: Environments-cmm.75.mcz

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

The Inbox: Environments-cmm.75.mcz

commits-2
Chris Muller uploaded a new version of Environments to project The Inbox:
http://source.squeak.org/inbox/Environments-cmm.75.mcz

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

Name: Environments-cmm.75
Author: cmm
Time: 17 February 2019, 8:54:02.934673 pm
UUID: 13aceb53-d0e1-4823-a86c-a2abacb6c2d9
Ancestors: Environments-pre.73

Keep and access Classes and Traits by their #name's logical value instead of the physical identity of the String object.

=============== Diff against Environments-pre.73 ===============

Item was removed:
- (PackageInfo named: 'Environments') preamble: '"Fix ''Instances'' entry for Smalltalk Environment."
- | dict |
- dict := (Environment classPool at: ''Instances'').
- dict keys
- do: [ : eachName | (eachName isSymbol not ) ifTrue: [ dict at: eachName asSymbol put: (dict removeKey: eachName) ] ].
-
- "Let Environment names be, consistently, Symbols."
- Environment allInstances do:
- [ : each |
- each info
- instVarNamed: ''name''
- put: (each name asSymbol) ]'!

Item was changed:
  ----- Method: Environment>>allClassesAndTraitsDo: (in category 'classes and traits') -----
  allClassesAndTraitsDo: aBlock
  declarations keysAndValuesDo:
  [:key :value |
+ ((value isBehavior) and: [key = value name]) ifTrue:
- ((value isBehavior) and: [key == value name]) ifTrue:
  [aBlock value: value]]!

Item was changed:
  ----- Method: Environment>>classOrTraitNamed: (in category 'classes and traits') -----
  classOrTraitNamed: aString
  "aString is either a class or trait name or a class or trait name followed by ' class' or 'classTrait' respectively.
  Answer the class or metaclass it names."
 
  | meta baseName |
  (aString endsWith: ' class')
  ifTrue: [meta := true.
  baseName := aString copyFrom: 1 to: aString size - 6]
  ifFalse: [
  (aString endsWith: ' classTrait')
  ifTrue: [
  meta := true.
  baseName := aString copyFrom: 1 to: aString size - 11]
  ifFalse: [
  meta := false.
  baseName := aString]].
 
+ ^declarations at: baseName ifPresent:
- ^declarations at: baseName asSymbol ifPresent:
  [ :global |
    global isBehavior ifTrue:
  [ meta
  ifFalse: [ global ]
  ifTrue: [ global classSide ]]]!

Item was changed:
  ----- Method: Environment>>hasClassNamed: (in category 'classes and traits') -----
+ hasClassNamed: aString
+ ^ declarations
+ at: aString
+ ifPresent: [ : global | global isKindOf: Class ]
+ ifAbsent: [ false ]!
- hasClassNamed: aString
- Symbol hasInterned: aString ifTrue:
- [:symbol |
- ^ (declarations at: symbol ifAbsent: [nil])
- isKindOf: Class].
- ^ false.!

Item was changed:
  ----- Method: Environment>>initialize (in category 'initialize-release') -----
  initialize
+ declarations := Dictionary new.
+ bindings := Dictionary new.
- declarations := IdentityDictionary new.
- bindings := IdentityDictionary new.
  undeclared := WeakIdentityDictionary new.
  policies := Array new.
  observers := IdentitySet new.!

Item was changed:
  ----- Method: Environment>>removeClassNamed: (in category 'classes and traits') -----
  removeClassNamed: aString
  declarations
+ at: aString
- at: aString asSymbol
  ifPresent: [:class | class removeFromSystem]
  ifAbsent:
  [Transcript cr; show: 'Removal of class named ', aString,
  ' ignored because ', aString, ' does not exist.']!

Item was added:
+ (PackageInfo named: 'Environments') postscript: '"Fix ''Instances'' entry for Smalltalk Environment."
+ | dict |
+ dict := (Environment classPool at: ''Instances'').
+ Environment classPool at: ''Instances'' put: (dict := dict as: Dictionary).
+
+ "Fix ''delcarations'' and ''bindings'' of existing Environments."
+ Environment allInstances do:
+ [ : each |
+ #(''declarations'' ''bindings'') do: [ : eachIdDictVar | (each instVarNamed: eachIdDictVar put: ((each instVarNamed: eachIdDictVar) as: Dictionary)) ] ]'!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Environments-cmm.75.mcz

Jakob Reschke
Previously, the intended invariant apparently was that all bindings/declarations, and in particular the ones for classes and traits, have a Symbol as their key. Since the identity hash is stored in an object's header and needs not be computed each time, this should benefit the lookup performance (although that might not be practically relevant for already loaded code because methods have direct references to the bindings for literals).

What is the need to deprecate that invariant? In my image, all non-meta classes have a Symbol as their #name.

Am Mo., 18. Feb. 2019 um 03:54 Uhr schrieb <[hidden email]>:
Chris Muller uploaded a new version of Environments to project The Inbox:
http://source.squeak.org/inbox/Environments-cmm.75.mcz

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

Name: Environments-cmm.75
Author: cmm
Time: 17 February 2019, 8:54:02.934673 pm
UUID: 13aceb53-d0e1-4823-a86c-a2abacb6c2d9
Ancestors: Environments-pre.73

Keep and access Classes and Traits by their #name's logical value instead of the physical identity of the String object.

=============== Diff against Environments-pre.73 ===============

Item was removed:
- (PackageInfo named: 'Environments') preamble: '"Fix ''Instances'' entry for Smalltalk Environment."
- | dict |
- dict := (Environment classPool at: ''Instances'').
- dict keys
-       do: [ : eachName | (eachName isSymbol not ) ifTrue: [ dict at: eachName asSymbol put: (dict removeKey: eachName) ] ].
-
- "Let Environment names be, consistently, Symbols."
- Environment allInstances do:
-       [ : each |
-       each info
-               instVarNamed: ''name''
-               put: (each name asSymbol) ]'!

Item was changed:
  ----- Method: Environment>>allClassesAndTraitsDo: (in category 'classes and traits') -----
  allClassesAndTraitsDo: aBlock
        declarations keysAndValuesDo:
                [:key :value |
+               ((value isBehavior) and: [key = value name]) ifTrue:
-               ((value isBehavior) and: [key == value name]) ifTrue:
                        [aBlock value: value]]!

Item was changed:
  ----- Method: Environment>>classOrTraitNamed: (in category 'classes and traits') -----
  classOrTraitNamed: aString
        "aString is either a class or trait name or a class or trait name followed by ' class' or 'classTrait' respectively.
        Answer the class or metaclass it names."

        | meta baseName |
        (aString endsWith: ' class')
                ifTrue: [meta := true.
                                baseName := aString copyFrom: 1 to: aString size - 6]
                ifFalse: [
                        (aString endsWith: ' classTrait')
                                ifTrue: [
                                        meta := true.
                                        baseName := aString copyFrom: 1 to: aString size - 11]
                                ifFalse: [
                                        meta := false.
                                        baseName := aString]].

+       ^declarations at: baseName ifPresent:
-       ^declarations at: baseName asSymbol ifPresent:
                [ :global |
                global isBehavior ifTrue:
                        [ meta
                                ifFalse: [ global ]
                                ifTrue: [ global classSide ]]]!

Item was changed:
  ----- Method: Environment>>hasClassNamed: (in category 'classes and traits') -----
+ hasClassNamed: aString
+       ^ declarations
+               at: aString
+               ifPresent: [ : global | global isKindOf: Class ]
+               ifAbsent: [ false ]!
- hasClassNamed: aString
-       Symbol hasInterned: aString ifTrue:
-               [:symbol |
-               ^ (declarations at: symbol ifAbsent: [nil])
-                       isKindOf: Class].
-       ^ false.!

Item was changed:
  ----- Method: Environment>>initialize (in category 'initialize-release') -----
  initialize
+       declarations := Dictionary new.
+       bindings := Dictionary new.
-       declarations := IdentityDictionary new.
-       bindings := IdentityDictionary new.
        undeclared := WeakIdentityDictionary new.
        policies := Array new.
        observers := IdentitySet new.!

Item was changed:
  ----- Method: Environment>>removeClassNamed: (in category 'classes and traits') -----
  removeClassNamed: aString
        declarations
+               at: aString
-               at: aString asSymbol
                ifPresent: [:class | class removeFromSystem]
                ifAbsent:
                        [Transcript cr; show: 'Removal of class named ', aString,
                        ' ignored because ', aString, ' does not exist.']!

Item was added:
+ (PackageInfo named: 'Environments') postscript: '"Fix ''Instances'' entry for Smalltalk Environment."
+ | dict |
+ dict := (Environment classPool at: ''Instances'').
+ Environment classPool at: ''Instances'' put: (dict := dict as: Dictionary).
+
+ "Fix ''delcarations'' and ''bindings'' of existing Environments."
+ Environment allInstances do:
+       [ : each |
+       #(''declarations'' ''bindings'') do: [ : eachIdDictVar | (each instVarNamed: eachIdDictVar put: ((each instVarNamed: eachIdDictVar) as: Dictionary)) ] ]'!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Environments-cmm.75.mcz

Chris Muller-3
Hi Jakob,

It's a violation of encapsulation to expose that implementation detail
to the outside and even require clients to be aware of it.  Not only
that but, in this case, it actually kills performance too, as shown in
this more detailed explanation from my prior post.

   http://lists.squeakfoundation.org/pipermail/squeak-dev/2018-November/200792.html

Best,
  Chris




On Mon, Apr 22, 2019 at 5:25 AM Jakob Reschke <[hidden email]> wrote:

>
> Previously, the intended invariant apparently was that all bindings/declarations, and in particular the ones for classes and traits, have a Symbol as their key. Since the identity hash is stored in an object's header and needs not be computed each time, this should benefit the lookup performance (although that might not be practically relevant for already loaded code because methods have direct references to the bindings for literals).
>
> What is the need to deprecate that invariant? In my image, all non-meta classes have a Symbol as their #name.
>
> Am Mo., 18. Feb. 2019 um 03:54 Uhr schrieb <[hidden email]>:
>>
>> Chris Muller uploaded a new version of Environments to project The Inbox:
>> http://source.squeak.org/inbox/Environments-cmm.75.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Environments-cmm.75
>> Author: cmm
>> Time: 17 February 2019, 8:54:02.934673 pm
>> UUID: 13aceb53-d0e1-4823-a86c-a2abacb6c2d9
>> Ancestors: Environments-pre.73
>>
>> Keep and access Classes and Traits by their #name's logical value instead of the physical identity of the String object.
>>
>> =============== Diff against Environments-pre.73 ===============
>>
>> Item was removed:
>> - (PackageInfo named: 'Environments') preamble: '"Fix ''Instances'' entry for Smalltalk Environment."
>> - | dict |
>> - dict := (Environment classPool at: ''Instances'').
>> - dict keys
>> -       do: [ : eachName | (eachName isSymbol not ) ifTrue: [ dict at: eachName asSymbol put: (dict removeKey: eachName) ] ].
>> -
>> - "Let Environment names be, consistently, Symbols."
>> - Environment allInstances do:
>> -       [ : each |
>> -       each info
>> -               instVarNamed: ''name''
>> -               put: (each name asSymbol) ]'!
>>
>> Item was changed:
>>   ----- Method: Environment>>allClassesAndTraitsDo: (in category 'classes and traits') -----
>>   allClassesAndTraitsDo: aBlock
>>         declarations keysAndValuesDo:
>>                 [:key :value |
>> +               ((value isBehavior) and: [key = value name]) ifTrue:
>> -               ((value isBehavior) and: [key == value name]) ifTrue:
>>                         [aBlock value: value]]!
>>
>> Item was changed:
>>   ----- Method: Environment>>classOrTraitNamed: (in category 'classes and traits') -----
>>   classOrTraitNamed: aString
>>         "aString is either a class or trait name or a class or trait name followed by ' class' or 'classTrait' respectively.
>>         Answer the class or metaclass it names."
>>
>>         | meta baseName |
>>         (aString endsWith: ' class')
>>                 ifTrue: [meta := true.
>>                                 baseName := aString copyFrom: 1 to: aString size - 6]
>>                 ifFalse: [
>>                         (aString endsWith: ' classTrait')
>>                                 ifTrue: [
>>                                         meta := true.
>>                                         baseName := aString copyFrom: 1 to: aString size - 11]
>>                                 ifFalse: [
>>                                         meta := false.
>>                                         baseName := aString]].
>>
>> +       ^declarations at: baseName ifPresent:
>> -       ^declarations at: baseName asSymbol ifPresent:
>>                 [ :global |
>>                 global isBehavior ifTrue:
>>                         [ meta
>>                                 ifFalse: [ global ]
>>                                 ifTrue: [ global classSide ]]]!
>>
>> Item was changed:
>>   ----- Method: Environment>>hasClassNamed: (in category 'classes and traits') -----
>> + hasClassNamed: aString
>> +       ^ declarations
>> +               at: aString
>> +               ifPresent: [ : global | global isKindOf: Class ]
>> +               ifAbsent: [ false ]!
>> - hasClassNamed: aString
>> -       Symbol hasInterned: aString ifTrue:
>> -               [:symbol |
>> -               ^ (declarations at: symbol ifAbsent: [nil])
>> -                       isKindOf: Class].
>> -       ^ false.!
>>
>> Item was changed:
>>   ----- Method: Environment>>initialize (in category 'initialize-release') -----
>>   initialize
>> +       declarations := Dictionary new.
>> +       bindings := Dictionary new.
>> -       declarations := IdentityDictionary new.
>> -       bindings := IdentityDictionary new.
>>         undeclared := WeakIdentityDictionary new.
>>         policies := Array new.
>>         observers := IdentitySet new.!
>>
>> Item was changed:
>>   ----- Method: Environment>>removeClassNamed: (in category 'classes and traits') -----
>>   removeClassNamed: aString
>>         declarations
>> +               at: aString
>> -               at: aString asSymbol
>>                 ifPresent: [:class | class removeFromSystem]
>>                 ifAbsent:
>>                         [Transcript cr; show: 'Removal of class named ', aString,
>>                         ' ignored because ', aString, ' does not exist.']!
>>
>> Item was added:
>> + (PackageInfo named: 'Environments') postscript: '"Fix ''Instances'' entry for Smalltalk Environment."
>> + | dict |
>> + dict := (Environment classPool at: ''Instances'').
>> + Environment classPool at: ''Instances'' put: (dict := dict as: Dictionary).
>> +
>> + "Fix ''delcarations'' and ''bindings'' of existing Environments."
>> + Environment allInstances do:
>> +       [ : each |
>> +       #(''declarations'' ''bindings'') do: [ : eachIdDictVar | (each instVarNamed: eachIdDictVar put: ((each instVarNamed: eachIdDictVar) as: Dictionary)) ] ]'!
>>
>>
>