The Inbox: Environments-ul.43.mcz
Locked
 
|
A new version of Environments was added to project The Inbox:
http://source.squeak.org/inbox/Environments-ul.43.mcz ==================== Summary ==================== Name: Environments-ul.43 Author: ul Time: 29 December 2013, 10:04:32.454 pm UUID: a91a7c62-f608-48aa-b296-2d541c0083f5 Ancestors: Environments-ul.42 Fixed a typo in the postscript. =============== Diff against Environments-ul.40 =============== Item was changed: ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating') ----- at: aSymbol ifAbsentPut: aBlock + + ^declarations - ^ declarations at: aSymbol + ifAbsent: [ self at: aSymbol put: aBlock value ]! - ifAbsentPut: aBlock! Item was changed: ----- Method: Environment>>at:put: (in category 'emulating') ----- at: aSymbol put: anObject + + | binding newBinding | + newBinding := aSymbol => anObject. + binding := declarations associationAt: aSymbol ifAbsent: [ nil ]. + binding ifNotNil: [ + binding class == newBinding class + ifTrue: [ binding value: anObject ] + ifFalse: [ binding becomeForward: newBinding ]. + ^anObject ]. + binding := undeclared associationAt: aSymbol ifAbsent: [ nil ]. + binding + ifNil: [ binding := newBinding ] + ifNotNil: [ + undeclared removeKey: aSymbol. + binding class == newBinding class + ifTrue: [ binding value: anObject ] + ifFalse: [ binding becomeForward: newBinding ] ]. + declarations add: binding. + references add: binding. + exports bind: binding. "Do we really want this?" + ^anObject - | binding | - (declarations includesKey: aSymbol) - ifTrue: [declarations at: aSymbol put: anObject] - ifFalse: - [(undeclared includesKey: aSymbol) - ifTrue: - [declarations declare: aSymbol from: undeclared. - declarations at: aSymbol put: anObject] - ifFalse: - [binding := aSymbol => anObject. - declarations add: binding. - exports bind: binding]]. - ^ anObject ! Item was changed: ----- Method: Environment>>removeKey:ifAbsent: (in category 'emulating') ----- removeKey: key ifAbsent: aBlock + + (declarations includesKey: key) ifFalse: [ ^aBlock value ]. + references removeKey: key ifAbsent: []. + public removeKey: key ifAbsent: []. + ^declarations removeKey: key! - self flag: #review. - ^ declarations removeKey: key ifAbsent: aBlock! Item was changed: + (PackageInfo named: 'Environments') postscript: '| globalsWithMultipleBindings | + "Fix exports." + Smalltalk globals exports instVarNamed: #namespace put: Smalltalk globals public. + "Fix imports." + (Smalltalk globals instVarNamed: #imports) instVarNamed: #next put: nil. + "Unify globals." + globalsWithMultipleBindings := Dictionary new. + Binding allSubInstances do: [ :binding | + (globalsWithMultipleBindings at: binding key ifAbsentPut: [ IdentitySet new ]) add: binding ]. + globalsWithMultipleBindings := globalsWithMultipleBindings select: [ :each | each size > 1 ]. + globalsWithMultipleBindings keysAndValuesDo: [ :name :set | + | realBinding | + realBinding := Smalltalk associationAt: name ifAbsent: [ nil ]. + realBinding ifNotNil: [ + set do: [ :each | + each == realBinding ifFalse: [ each becomeForward: realBinding ] ] ] ]'! - (PackageInfo named: 'Environments') postscript: '"below, add code to be run after the loading of this package" - - | allAliases toBeRecompiled undesirableAliases | - - "Collect the CompiledMethods pointing to an Alias" - allAliases := Alias allInstances. - toBeRecompiled := CompiledMethod allInstances select: [:c | c isInstalled and: [allAliases anySatisfy: [:a | c pointsTo: a]]]. - - "Collect the Aliases pointing to some class binding in the same Environment with same name" - undesirableAliases := (Smalltalk globals instVarNamed: ''references'') associations select: [:e | - e class = Alias and: [e key = e source key - and: [(Smalltalk globals instVarNamed: ''declarations'') associations includes: e source]]]. - - "Replace the undesirable Aliases with their source binding" - undesirableAliases do: [:a | a becomeForward: a source]. - - "Rehash the references because they pointed to those Aliases - hope there''s nothing else to rehash" - (Smalltalk globals instVarNamed: ''references'') rehash. - - "Recompile the CompiledMethod that used to point to an Alias, because the bytecodes do change" - Symbol rehash. - toBeRecompiled do: [:c | c methodClass recompile: c selector]. - - allAliases := toBeRecompiled := undesirableAliases := nil. - Smalltalk garbageCollect. - Alias allInstances size. - '! |
Re: The Inbox: Environments-ul.43.mcz
|
|
I've uploaded this version to the Inbox to let you review it before I push
it to the Trunk. It fixes the most urgent issues of Environments, but it also includes some "radical" changes, and a "high impact" postscript. Levente On Sun, 29 Dec 2013, [hidden email] wrote: > A new version of Environments was added to project The Inbox: > http://source.squeak.org/inbox/Environments-ul.43.mcz > > ==================== Summary ==================== > > Name: Environments-ul.43 > Author: ul > Time: 29 December 2013, 10:04:32.454 pm > UUID: a91a7c62-f608-48aa-b296-2d541c0083f5 > Ancestors: Environments-ul.42 > > Fixed a typo in the postscript. > > =============== Diff against Environments-ul.40 =============== > > Item was changed: > ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating') ----- > at: aSymbol ifAbsentPut: aBlock > + > + ^declarations > - ^ declarations > at: aSymbol > + ifAbsent: [ self at: aSymbol put: aBlock value ]! > - ifAbsentPut: aBlock! > > Item was changed: > ----- Method: Environment>>at:put: (in category 'emulating') ----- > at: aSymbol put: anObject > + > + | binding newBinding | > + newBinding := aSymbol => anObject. > + binding := declarations associationAt: aSymbol ifAbsent: [ nil ]. > + binding ifNotNil: [ > + binding class == newBinding class > + ifTrue: [ binding value: anObject ] > + ifFalse: [ binding becomeForward: newBinding ]. > + ^anObject ]. > + binding := undeclared associationAt: aSymbol ifAbsent: [ nil ]. > + binding > + ifNil: [ binding := newBinding ] > + ifNotNil: [ > + undeclared removeKey: aSymbol. > + binding class == newBinding class > + ifTrue: [ binding value: anObject ] > + ifFalse: [ binding becomeForward: newBinding ] ]. > + declarations add: binding. > + references add: binding. > + exports bind: binding. "Do we really want this?" > + ^anObject > - | binding | > - (declarations includesKey: aSymbol) > - ifTrue: [declarations at: aSymbol put: anObject] > - ifFalse: > - [(undeclared includesKey: aSymbol) > - ifTrue: > - [declarations declare: aSymbol from: undeclared. > - declarations at: aSymbol put: anObject] > - ifFalse: > - [binding := aSymbol => anObject. > - declarations add: binding. > - exports bind: binding]]. > - ^ anObject > ! > > Item was changed: > ----- Method: Environment>>removeKey:ifAbsent: (in category 'emulating') ----- > removeKey: key ifAbsent: aBlock > + > + (declarations includesKey: key) ifFalse: [ ^aBlock value ]. > + references removeKey: key ifAbsent: []. > + public removeKey: key ifAbsent: []. > + ^declarations removeKey: key! > - self flag: #review. > - ^ declarations removeKey: key ifAbsent: aBlock! > > Item was changed: > + (PackageInfo named: 'Environments') postscript: '| globalsWithMultipleBindings | > + "Fix exports." > + Smalltalk globals exports instVarNamed: #namespace put: Smalltalk globals public. > + "Fix imports." > + (Smalltalk globals instVarNamed: #imports) instVarNamed: #next put: nil. > + "Unify globals." > + globalsWithMultipleBindings := Dictionary new. > + Binding allSubInstances do: [ :binding | > + (globalsWithMultipleBindings at: binding key ifAbsentPut: [ IdentitySet new ]) add: binding ]. > + globalsWithMultipleBindings := globalsWithMultipleBindings select: [ :each | each size > 1 ]. > + globalsWithMultipleBindings keysAndValuesDo: [ :name :set | > + | realBinding | > + realBinding := Smalltalk associationAt: name ifAbsent: [ nil ]. > + realBinding ifNotNil: [ > + set do: [ :each | > + each == realBinding ifFalse: [ each becomeForward: realBinding ] ] ] ]'! > - (PackageInfo named: 'Environments') postscript: '"below, add code to be run after the loading of this package" > - > - | allAliases toBeRecompiled undesirableAliases | > - > - "Collect the CompiledMethods pointing to an Alias" > - allAliases := Alias allInstances. > - toBeRecompiled := CompiledMethod allInstances select: [:c | c isInstalled and: [allAliases anySatisfy: [:a | c pointsTo: a]]]. > - > - "Collect the Aliases pointing to some class binding in the same Environment with same name" > - undesirableAliases := (Smalltalk globals instVarNamed: ''references'') associations select: [:e | > - e class = Alias and: [e key = e source key > - and: [(Smalltalk globals instVarNamed: ''declarations'') associations includes: e source]]]. > - > - "Replace the undesirable Aliases with their source binding" > - undesirableAliases do: [:a | a becomeForward: a source]. > - > - "Rehash the references because they pointed to those Aliases - hope there''s nothing else to rehash" > - (Smalltalk globals instVarNamed: ''references'') rehash. > - > - "Recompile the CompiledMethod that used to point to an Alias, because the bytecodes do change" > - Symbol rehash. > - toBeRecompiled do: [:c | c methodClass recompile: c selector]. > - > - allAliases := toBeRecompiled := undesirableAliases := nil. > - Smalltalk garbageCollect. > - Alias allInstances size. > - '! > > > |
Re: The Inbox: Environments-ul.43.mcz
|
|
"Radical" would be the #becomeForwards: calls?
If I read it right, this commit "just" updates the various caches (references, exports), and updates undeclared, when something's added to the Environment. It then also ensures that _removed_ bindings are removed from the various caches. Should #removeKey:ifAbsent: possibly update undeclared? Couldn't you still have CompiledMethods floating around holding references to a class you just deleted from the Environment? Lastly, the postscript runs over all globals, finds those globals with more than one binding, and forcibly unifies them all (through #becomeForward:) to point to whatever Smalltalk globals points to. I'd be interested in seeing tests exercising these changes directly, but I realise that partly they will be, by virtue of making the current test suite work. frank On 29 December 2013 21:28, Levente Uzonyi <[hidden email]> wrote: > I've uploaded this version to the Inbox to let you review it before I push > it to the Trunk. It fixes the most urgent issues of Environments, but it > also includes some "radical" changes, and a "high impact" postscript. > > > Levente > > > On Sun, 29 Dec 2013, [hidden email] wrote: > >> A new version of Environments was added to project The Inbox: >> http://source.squeak.org/inbox/Environments-ul.43.mcz >> >> ==================== Summary ==================== >> >> Name: Environments-ul.43 >> Author: ul >> Time: 29 December 2013, 10:04:32.454 pm >> UUID: a91a7c62-f608-48aa-b296-2d541c0083f5 >> Ancestors: Environments-ul.42 >> >> Fixed a typo in the postscript. >> >> =============== Diff against Environments-ul.40 =============== >> >> Item was changed: >> ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating') >> ----- >> at: aSymbol ifAbsentPut: aBlock >> + >> + ^declarations >> - ^ declarations >> at: aSymbol >> + ifAbsent: [ self at: aSymbol put: aBlock value ]! >> - ifAbsentPut: aBlock! >> >> Item was changed: >> ----- Method: Environment>>at:put: (in category 'emulating') ----- >> at: aSymbol put: anObject >> + >> + | binding newBinding | >> + newBinding := aSymbol => anObject. >> + binding := declarations associationAt: aSymbol ifAbsent: [ nil ]. >> + binding ifNotNil: [ >> + binding class == newBinding class >> + ifTrue: [ binding value: anObject ] >> + ifFalse: [ binding becomeForward: newBinding ]. >> + ^anObject ]. >> + binding := undeclared associationAt: aSymbol ifAbsent: [ nil ]. >> + binding >> + ifNil: [ binding := newBinding ] >> + ifNotNil: [ >> + undeclared removeKey: aSymbol. >> + binding class == newBinding class >> + ifTrue: [ binding value: anObject ] >> + ifFalse: [ binding becomeForward: >> newBinding ] ]. >> + declarations add: binding. >> + references add: binding. >> + exports bind: binding. "Do we really want this?" >> + ^anObject >> - | binding | >> - (declarations includesKey: aSymbol) >> - ifTrue: [declarations at: aSymbol put: anObject] >> - ifFalse: >> - [(undeclared includesKey: aSymbol) >> - ifTrue: >> - [declarations declare: aSymbol >> from: undeclared. >> - declarations at: aSymbol put: >> anObject] >> - ifFalse: >> - [binding := aSymbol => anObject. >> - declarations add: binding. >> - exports bind: binding]]. >> - ^ anObject >> ! >> >> Item was changed: >> ----- Method: Environment>>removeKey:ifAbsent: (in category 'emulating') >> ----- >> removeKey: key ifAbsent: aBlock >> + >> + (declarations includesKey: key) ifFalse: [ ^aBlock value ]. >> + references removeKey: key ifAbsent: []. >> + public removeKey: key ifAbsent: []. >> + ^declarations removeKey: key! >> - self flag: #review. >> - ^ declarations removeKey: key ifAbsent: aBlock! >> >> Item was changed: >> + (PackageInfo named: 'Environments') postscript: '| >> globalsWithMultipleBindings | >> + "Fix exports." >> + Smalltalk globals exports instVarNamed: #namespace put: Smalltalk >> globals public. >> + "Fix imports." >> + (Smalltalk globals instVarNamed: #imports) instVarNamed: #next put: nil. >> + "Unify globals." >> + globalsWithMultipleBindings := Dictionary new. >> + Binding allSubInstances do: [ :binding | >> + (globalsWithMultipleBindings at: binding key ifAbsentPut: [ >> IdentitySet new ]) add: binding ]. >> + globalsWithMultipleBindings := globalsWithMultipleBindings select: [ >> :each | each size > 1 ]. >> + globalsWithMultipleBindings keysAndValuesDo: [ :name :set | >> + | realBinding | >> + realBinding := Smalltalk associationAt: name ifAbsent: [ nil ]. >> + realBinding ifNotNil: [ >> + set do: [ :each | >> + each == realBinding ifFalse: [ each becomeForward: >> realBinding ] ] ] ]'! >> - (PackageInfo named: 'Environments') postscript: '"below, add code to be >> run after the loading of this package" >> - >> - | allAliases toBeRecompiled undesirableAliases | >> - >> - "Collect the CompiledMethods pointing to an Alias" >> - allAliases := Alias allInstances. >> - toBeRecompiled := CompiledMethod allInstances select: [:c | c >> isInstalled and: [allAliases anySatisfy: [:a | c pointsTo: a]]]. >> - >> - "Collect the Aliases pointing to some class binding in the same >> Environment with same name" >> - undesirableAliases := (Smalltalk globals instVarNamed: ''references'') >> associations select: [:e | >> - e class = Alias and: [e key = e source key >> - and: [(Smalltalk globals instVarNamed: ''declarations'') >> associations includes: e source]]]. >> - >> - "Replace the undesirable Aliases with their source binding" >> - undesirableAliases do: [:a | a becomeForward: a source]. >> - >> - "Rehash the references because they pointed to those Aliases - hope >> there''s nothing else to rehash" >> - (Smalltalk globals instVarNamed: ''references'') rehash. >> - >> - "Recompile the CompiledMethod that used to point to an Alias, because >> the bytecodes do change" >> - Symbol rehash. >> - toBeRecompiled do: [:c | c methodClass recompile: c selector]. >> - >> - allAliases := toBeRecompiled := undesirableAliases := nil. >> - Smalltalk garbageCollect. >> - Alias allInstances size. >> - '! >> >> >> > |
Re: The Inbox: Environments-ul.43.mcz
|
|
On Sun, 29 Dec 2013, Frank Shearar wrote:
> "Radical" would be the #becomeForwards: calls? No. Radical in the sense that those three methods are not just forwarders anymore. A bug in any of them can have bad consequences. > > If I read it right, this commit "just" updates the various caches > (references, exports), and updates undeclared, when something's added > to the Environment. It then also ensures that _removed_ bindings are > removed from the various caches. Should #removeKey:ifAbsent: possibly > update undeclared? Couldn't you still have CompiledMethods floating > around holding references to a class you just deleted from the > Environment? No, code which uses #removeKey:ifAbsent: also updates Undeclared. > > Lastly, the postscript runs over all globals, finds those globals with > more than one binding, and forcibly unifies them all (through > #becomeForward:) to point to whatever Smalltalk globals points to. The postscript also fixes the exports and cleans the imports. > > I'd be interested in seeing tests exercising these changes directly, > but I realise that partly they will be, by virtue of making the > current test suite work. We definitely need more tests for Environments. Levente > > frank > > On 29 December 2013 21:28, Levente Uzonyi <[hidden email]> wrote: >> I've uploaded this version to the Inbox to let you review it before I push >> it to the Trunk. It fixes the most urgent issues of Environments, but it >> also includes some "radical" changes, and a "high impact" postscript. >> >> >> Levente >> >> >> On Sun, 29 Dec 2013, [hidden email] wrote: >> >>> A new version of Environments was added to project The Inbox: >>> http://source.squeak.org/inbox/Environments-ul.43.mcz >>> >>> ==================== Summary ==================== >>> >>> Name: Environments-ul.43 >>> Author: ul >>> Time: 29 December 2013, 10:04:32.454 pm >>> UUID: a91a7c62-f608-48aa-b296-2d541c0083f5 >>> Ancestors: Environments-ul.42 >>> >>> Fixed a typo in the postscript. >>> >>> =============== Diff against Environments-ul.40 =============== >>> >>> Item was changed: >>> ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating') >>> ----- >>> at: aSymbol ifAbsentPut: aBlock >>> + >>> + ^declarations >>> - ^ declarations >>> at: aSymbol >>> + ifAbsent: [ self at: aSymbol put: aBlock value ]! >>> - ifAbsentPut: aBlock! >>> >>> Item was changed: >>> ----- Method: Environment>>at:put: (in category 'emulating') ----- >>> at: aSymbol put: anObject >>> + >>> + | binding newBinding | >>> + newBinding := aSymbol => anObject. >>> + binding := declarations associationAt: aSymbol ifAbsent: [ nil ]. >>> + binding ifNotNil: [ >>> + binding class == newBinding class >>> + ifTrue: [ binding value: anObject ] >>> + ifFalse: [ binding becomeForward: newBinding ]. >>> + ^anObject ]. >>> + binding := undeclared associationAt: aSymbol ifAbsent: [ nil ]. >>> + binding >>> + ifNil: [ binding := newBinding ] >>> + ifNotNil: [ >>> + undeclared removeKey: aSymbol. >>> + binding class == newBinding class >>> + ifTrue: [ binding value: anObject ] >>> + ifFalse: [ binding becomeForward: >>> newBinding ] ]. >>> + declarations add: binding. >>> + references add: binding. >>> + exports bind: binding. "Do we really want this?" >>> + ^anObject >>> - | binding | >>> - (declarations includesKey: aSymbol) >>> - ifTrue: [declarations at: aSymbol put: anObject] >>> - ifFalse: >>> - [(undeclared includesKey: aSymbol) >>> - ifTrue: >>> - [declarations declare: aSymbol >>> from: undeclared. >>> - declarations at: aSymbol put: >>> anObject] >>> - ifFalse: >>> - [binding := aSymbol => anObject. >>> - declarations add: binding. >>> - exports bind: binding]]. >>> - ^ anObject >>> ! >>> >>> Item was changed: >>> ----- Method: Environment>>removeKey:ifAbsent: (in category 'emulating') >>> ----- >>> removeKey: key ifAbsent: aBlock >>> + >>> + (declarations includesKey: key) ifFalse: [ ^aBlock value ]. >>> + references removeKey: key ifAbsent: []. >>> + public removeKey: key ifAbsent: []. >>> + ^declarations removeKey: key! >>> - self flag: #review. >>> - ^ declarations removeKey: key ifAbsent: aBlock! >>> >>> Item was changed: >>> + (PackageInfo named: 'Environments') postscript: '| >>> globalsWithMultipleBindings | >>> + "Fix exports." >>> + Smalltalk globals exports instVarNamed: #namespace put: Smalltalk >>> globals public. >>> + "Fix imports." >>> + (Smalltalk globals instVarNamed: #imports) instVarNamed: #next put: nil. >>> + "Unify globals." >>> + globalsWithMultipleBindings := Dictionary new. >>> + Binding allSubInstances do: [ :binding | >>> + (globalsWithMultipleBindings at: binding key ifAbsentPut: [ >>> IdentitySet new ]) add: binding ]. >>> + globalsWithMultipleBindings := globalsWithMultipleBindings select: [ >>> :each | each size > 1 ]. >>> + globalsWithMultipleBindings keysAndValuesDo: [ :name :set | >>> + | realBinding | >>> + realBinding := Smalltalk associationAt: name ifAbsent: [ nil ]. >>> + realBinding ifNotNil: [ >>> + set do: [ :each | >>> + each == realBinding ifFalse: [ each becomeForward: >>> realBinding ] ] ] ]'! >>> - (PackageInfo named: 'Environments') postscript: '"below, add code to be >>> run after the loading of this package" >>> - >>> - | allAliases toBeRecompiled undesirableAliases | >>> - >>> - "Collect the CompiledMethods pointing to an Alias" >>> - allAliases := Alias allInstances. >>> - toBeRecompiled := CompiledMethod allInstances select: [:c | c >>> isInstalled and: [allAliases anySatisfy: [:a | c pointsTo: a]]]. >>> - >>> - "Collect the Aliases pointing to some class binding in the same >>> Environment with same name" >>> - undesirableAliases := (Smalltalk globals instVarNamed: ''references'') >>> associations select: [:e | >>> - e class = Alias and: [e key = e source key >>> - and: [(Smalltalk globals instVarNamed: ''declarations'') >>> associations includes: e source]]]. >>> - >>> - "Replace the undesirable Aliases with their source binding" >>> - undesirableAliases do: [:a | a becomeForward: a source]. >>> - >>> - "Rehash the references because they pointed to those Aliases - hope >>> there''s nothing else to rehash" >>> - (Smalltalk globals instVarNamed: ''references'') rehash. >>> - >>> - "Recompile the CompiledMethod that used to point to an Alias, because >>> the bytecodes do change" >>> - Symbol rehash. >>> - toBeRecompiled do: [:c | c methodClass recompile: c selector]. >>> - >>> - allAliases := toBeRecompiled := undesirableAliases := nil. >>> - Smalltalk garbageCollect. >>> - Alias allInstances size. >>> - '! >>> >>> >>> >> > > |
Re: The Inbox: Environments-ul.43.mcz
|
|
In reply to this post by commits-2
On Sun, Dec 29, 2013 at 4:15 PM, <[hidden email]> wrote:
Item was changed: Yup. We want to make use of the new become logic in #at:put: here too... Item was changed: + references add: binding. I think the above line is problematic. It by by-passes all the import/export logic. + exports bind: binding. "Do we really want this?" Yes. + ^anObject Again, the above line by-passes all the import/export logic. Colin |
Re: The Inbox: Environments-ul.43.mcz
|
|
In reply to this post by Levente Uzonyi-2
On Sun, Dec 29, 2013 at 4:28 PM, Levente Uzonyi <[hidden email]> wrote: I've uploaded this version to the Inbox to let you review it before I push it to the Trunk. It fixes the most urgent issues of Environments, but it also includes some "radical" changes, and a "high impact" postscript. Hi Levante, I've been working on this too. My changes are even more radical—they include a rewrite of the import/export system to build up the reference dictionary eagerly, rather than lazily. I like the #becomeForward: stuff you've done; I'll adapt them to the new import system. There's still some kinks to work out, but I'll post it to the Inbox tomorrow.
Colin |
Re: The Inbox: Environments-ul.43.mcz
|
|
In reply to this post by Levente Uzonyi-2
On 29 December 2013 23:33, Levente Uzonyi <[hidden email]> wrote:
> On Sun, 29 Dec 2013, Frank Shearar wrote: > >> "Radical" would be the #becomeForwards: calls? > > > No. Radical in the sense that those three methods are not just forwarders > anymore. A bug in any of them can have bad consequences. Right. >> If I read it right, this commit "just" updates the various caches >> (references, exports), and updates undeclared, when something's added >> to the Environment. It then also ensures that _removed_ bindings are >> removed from the various caches. Should #removeKey:ifAbsent: possibly >> update undeclared? Couldn't you still have CompiledMethods floating >> around holding references to a class you just deleted from the >> Environment? > > > No, code which uses #removeKey:ifAbsent: also updates Undeclared. Er, of course, because had I been thinking properly, I'd have realised that the receiver is an Environment, not an IdentityDictionary. >> Lastly, the postscript runs over all globals, finds those globals with >> more than one binding, and forcibly unifies them all (through >> #becomeForward:) to point to whatever Smalltalk globals points to. > > > The postscript also fixes the exports and cleans the imports. > > >> >> I'd be interested in seeing tests exercising these changes directly, >> but I realise that partly they will be, by virtue of making the >> current test suite work. > > > We definitely need more tests for Environments. > > > Levente > > >> >> frank >> >> On 29 December 2013 21:28, Levente Uzonyi <[hidden email]> wrote: >>> >>> I've uploaded this version to the Inbox to let you review it before I >>> push >>> it to the Trunk. It fixes the most urgent issues of Environments, but it >>> also includes some "radical" changes, and a "high impact" postscript. >>> >>> >>> Levente >>> >>> >>> On Sun, 29 Dec 2013, [hidden email] wrote: >>> >>>> A new version of Environments was added to project The Inbox: >>>> http://source.squeak.org/inbox/Environments-ul.43.mcz >>>> >>>> ==================== Summary ==================== >>>> >>>> Name: Environments-ul.43 >>>> Author: ul >>>> Time: 29 December 2013, 10:04:32.454 pm >>>> UUID: a91a7c62-f608-48aa-b296-2d541c0083f5 >>>> Ancestors: Environments-ul.42 >>>> >>>> Fixed a typo in the postscript. >>>> >>>> =============== Diff against Environments-ul.40 =============== >>>> >>>> Item was changed: >>>> ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating') >>>> ----- >>>> at: aSymbol ifAbsentPut: aBlock >>>> + >>>> + ^declarations >>>> - ^ declarations >>>> at: aSymbol >>>> + ifAbsent: [ self at: aSymbol put: aBlock value ]! >>>> - ifAbsentPut: aBlock! >>>> >>>> Item was changed: >>>> ----- Method: Environment>>at:put: (in category 'emulating') ----- >>>> at: aSymbol put: anObject >>>> + >>>> + | binding newBinding | >>>> + newBinding := aSymbol => anObject. >>>> + binding := declarations associationAt: aSymbol ifAbsent: [ nil >>>> ]. >>>> + binding ifNotNil: [ >>>> + binding class == newBinding class >>>> + ifTrue: [ binding value: anObject ] >>>> + ifFalse: [ binding becomeForward: newBinding ]. >>>> + ^anObject ]. >>>> + binding := undeclared associationAt: aSymbol ifAbsent: [ nil ]. >>>> + binding >>>> + ifNil: [ binding := newBinding ] >>>> + ifNotNil: [ >>>> + undeclared removeKey: aSymbol. >>>> + binding class == newBinding class >>>> + ifTrue: [ binding value: anObject ] >>>> + ifFalse: [ binding becomeForward: >>>> newBinding ] ]. >>>> + declarations add: binding. >>>> + references add: binding. >>>> + exports bind: binding. "Do we really want this?" >>>> + ^anObject >>>> - | binding | >>>> - (declarations includesKey: aSymbol) >>>> - ifTrue: [declarations at: aSymbol put: anObject] >>>> - ifFalse: >>>> - [(undeclared includesKey: aSymbol) >>>> - ifTrue: >>>> - [declarations declare: aSymbol >>>> from: undeclared. >>>> - declarations at: aSymbol put: >>>> anObject] >>>> - ifFalse: >>>> - [binding := aSymbol => anObject. >>>> - declarations add: binding. >>>> - exports bind: binding]]. >>>> - ^ anObject >>>> ! >>>> >>>> Item was changed: >>>> ----- Method: Environment>>removeKey:ifAbsent: (in category >>>> 'emulating') >>>> ----- >>>> removeKey: key ifAbsent: aBlock >>>> + >>>> + (declarations includesKey: key) ifFalse: [ ^aBlock value ]. >>>> + references removeKey: key ifAbsent: []. >>>> + public removeKey: key ifAbsent: []. >>>> + ^declarations removeKey: key! >>>> - self flag: #review. >>>> - ^ declarations removeKey: key ifAbsent: aBlock! >>>> >>>> Item was changed: >>>> + (PackageInfo named: 'Environments') postscript: '| >>>> globalsWithMultipleBindings | >>>> + "Fix exports." >>>> + Smalltalk globals exports instVarNamed: #namespace put: Smalltalk >>>> globals public. >>>> + "Fix imports." >>>> + (Smalltalk globals instVarNamed: #imports) instVarNamed: #next put: >>>> nil. >>>> + "Unify globals." >>>> + globalsWithMultipleBindings := Dictionary new. >>>> + Binding allSubInstances do: [ :binding | >>>> + (globalsWithMultipleBindings at: binding key ifAbsentPut: [ >>>> IdentitySet new ]) add: binding ]. >>>> + globalsWithMultipleBindings := globalsWithMultipleBindings select: [ >>>> :each | each size > 1 ]. >>>> + globalsWithMultipleBindings keysAndValuesDo: [ :name :set | >>>> + | realBinding | >>>> + realBinding := Smalltalk associationAt: name ifAbsent: [ nil ]. >>>> + realBinding ifNotNil: [ >>>> + set do: [ :each | >>>> + each == realBinding ifFalse: [ each >>>> becomeForward: >>>> realBinding ] ] ] ]'! >>>> - (PackageInfo named: 'Environments') postscript: '"below, add code to >>>> be >>>> run after the loading of this package" >>>> - >>>> - | allAliases toBeRecompiled undesirableAliases | >>>> - >>>> - "Collect the CompiledMethods pointing to an Alias" >>>> - allAliases := Alias allInstances. >>>> - toBeRecompiled := CompiledMethod allInstances select: [:c | c >>>> isInstalled and: [allAliases anySatisfy: [:a | c pointsTo: a]]]. >>>> - >>>> - "Collect the Aliases pointing to some class binding in the same >>>> Environment with same name" >>>> - undesirableAliases := (Smalltalk globals instVarNamed: ''references'') >>>> associations select: [:e | >>>> - e class = Alias and: [e key = e source key >>>> - and: [(Smalltalk globals instVarNamed: ''declarations'') >>>> associations includes: e source]]]. >>>> - >>>> - "Replace the undesirable Aliases with their source binding" >>>> - undesirableAliases do: [:a | a becomeForward: a source]. >>>> - >>>> - "Rehash the references because they pointed to those Aliases - hope >>>> there''s nothing else to rehash" >>>> - (Smalltalk globals instVarNamed: ''references'') rehash. >>>> - >>>> - "Recompile the CompiledMethod that used to point to an Alias, because >>>> the bytecodes do change" >>>> - Symbol rehash. >>>> - toBeRecompiled do: [:c | c methodClass recompile: c selector]. >>>> - >>>> - allAliases := toBeRecompiled := undesirableAliases := nil. >>>> - Smalltalk garbageCollect. >>>> - Alias allInstances size. >>>> - '! |
Re: The Inbox: Environments-ul.43.mcz
|
|
In reply to this post by Colin Putney-3
On Mon, 30 Dec 2013, Colin Putney wrote: > > > > On Sun, Dec 29, 2013 at 4:15 PM, <[hidden email]> wrote: > > Item was changed: > ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating') ----- > at: aSymbol ifAbsentPut: aBlock > + > + ^declarations > - ^ declarations > at: aSymbol > + ifAbsent: [ self at: aSymbol put: aBlock value ]! > - ifAbsentPut: aBlock! > > > > Yup. We want to make use of the new become logic in #at:put: here too... > > > Item was changed: > ----- Method: Environment>>at:put: (in category 'emulating') ----- > at: aSymbol put: anObject > + > + | binding newBinding | > + newBinding := aSymbol => anObject. > + binding := declarations associationAt: aSymbol ifAbsent: [ nil ]. > + binding ifNotNil: [ > + binding class == newBinding class > + ifTrue: [ binding value: anObject ] > + ifFalse: [ binding becomeForward: newBinding ]. > + ^anObject ]. > + binding := undeclared associationAt: aSymbol ifAbsent: [ nil ]. > + binding > + ifNil: [ binding := newBinding ] > + ifNotNil: [ > + undeclared removeKey: aSymbol. > + binding class == newBinding class > + ifTrue: [ binding value: anObject ] > + ifFalse: [ binding becomeForward: newBinding ] ]. > + declarations add: binding. > > + references add: binding. > > > I think the above line is problematic. It by by-passes all the import/export logic. class to shadow a class defined in your environment? > > > + exports bind: binding. "Do we really want this?" > > > Yes. > > > + ^anObject > > Item was changed: > ----- Method: Environment>>removeKey:ifAbsent: (in category 'emulating') ----- > removeKey: key ifAbsent: aBlock > + > + (declarations includesKey: key) ifFalse: [ ^aBlock value ]. > + references removeKey: key ifAbsent: []. > > > Again, the above line by-passes all the import/export logic. Levente > > > Colin > > > > |
Re: The Inbox: Environments-ul.43.mcz
|
|
On Mon, Dec 30, 2013 at 11:49 AM, Levente Uzonyi <[hidden email]> wrote:
Yes, if that's the way you've set up your imports.
|
Re: The Inbox: Environments-ul.43.mcz
|
|
On Mon, 30 Dec 2013, Colin Putney wrote:
> > > > On Mon, Dec 30, 2013 at 11:49 AM, Levente Uzonyi <[hidden email]> wrote: > > I think the above line is problematic. It by by-passes all the import/export logic. > > You're right, but this is somewhat intentional. Or do you want an imported class to shadow a class defined in your environment? > > > Yes, if that's the way you've set up your imports. > > See http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-December/175497.html environment E with a class C, but I've imported the environment IE, which also has a class C, and the C from IE shadows the C from E, then either we shouldn't allow renaming C from E, or we should change the renaming code to not modify references in E in this case. Levente > > > > > |
Re: The Inbox: Environments-ul.43.mcz
|
|
On Mon, Dec 30, 2013 at 12:20 PM, Levente Uzonyi <[hidden email]> wrote:
Yeah. That's actually a special case of the more general problem of adding or removing bindings during development. If you removed C from IE, then suddenly the C in E becomes visible, and methods compiled in E should be rebound. It's something that should be handled at the tools level rather than down in the guts of Environment. Per Tim's suggestion, we can throw notifications when this happens, and leave it up to the tools, load-script or whatever to deal with them.
One way we might handle it at the tools level is to have a conflict browser. It would show names which have more than one binding in a given environment, and the classes/globals they could be bound to. There would be commands for choosing which one to expose to your environment, and would automatically rejigger the imports to make that happen.
Similarly, the "references to it" browser should pay attention to the actual value being bound, rather than name it's bound to, or the actual binding in the CompiledMethod.
Colin |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |