The Trunk: Monticello-bf.504.mcz

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

The Trunk: Monticello-bf.504.mcz

commits-2
Bert Freudenberg uploaded a new version of Monticello to project The Trunk:
http://source.squeak.org/trunk/Monticello-bf.504.mcz

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

Name: Monticello-bf.504
Author: bf
Time: 19 April 2012, 9:36:55.211 am
UUID: 5fa1177d-b1ed-4cf0-8628-229f171ca814
Ancestors: Monticello-eem.503

Fix intermittant load problem. We sometimes would get a 'changes file not found' in the middle of updating, but retrying found it just fine. I finally tracked it down to too many files being open. This change re-uses the same read-only copy during a package load (which additionally might speed up loading a tiny bit).

=============== Diff against Monticello-eem.503 ===============

Item was changed:
  ----- Method: MCPackageLoader>>basicLoad (in category 'private') -----
  basicLoad
  "Load the contents of some package. This is the core loading method
  in Monticello. Be wary about modifying it unless you understand the details
  and dependencies of the various entities being modified."
  | pkgName |
  errorDefinitions := OrderedCollection new.
  "Obviously this isn't the package name but we don't have anything else
  to use here. ChangeSet current name will generally work since a CS is
  usually installed prior to installation."
  pkgName := ChangeSet current name.
 
+ [CurrentReadOnlySourceFiles cacheDuring: [[
+ "Pass 1: Load everything but the methods,  which are collected in methodAdditions."
- [["Pass 1: Load everything but the methods,  which are collected in methodAdditions."
  additions do: [:ea |
  ea isMethodDefinition
  ifTrue:[methodAdditions add: ea asMethodAddition]
  ifFalse:[[ea load]on: Error do: [errorDefinitions add: ea]].
  ] displayingProgress: 'Reshaping ', pkgName.
 
  "Try again any delayed definitions"
  self shouldWarnAboutErrors ifTrue: [self warnAboutErrors].
  errorDefinitions do: [:ea | ea load]
  displayingProgress: 'Reloading ', pkgName.
 
  "Pass 2: We compile new / changed methods"
  methodAdditions do:[:ea| ea createCompiledMethod]
  displayingProgress: 'Compiling ', pkgName.
 
  'Installing ', pkgName displayProgressFrom: 0 to: 2 during:[:bar|
  "There is no progress *during* installation since a progress bar update
  will redraw the world and potentially call methods that we're just trying to install."
  bar value: 1.
 
  "Pass 3: Install the new / changed methods
  (this is a separate pass to allow compiler changes to be loaded)"
  methodAdditions do:[:ea| ea installMethod].
 
  "Pass 4: Remove the obsolete methods"
  removals do:[:ea| ea unload].
  ].
 
  "Finally, notify observers for the method additions"
  methodAdditions do: [:each | each notifyObservers]
  "the message is fake but actually telling people how much time we spend
  in the notifications is embarrassing so lie instead"
  displayingProgress: 'Installing ', pkgName.
 
  additions do: [:ea | ea postloadOver: (self obsoletionFor: ea)]
  displayingProgress: 'Initializing ', pkgName.
 
  ] on: InMidstOfFileinNotification do: [:n | n resume: true]
+ ]] ensure: [self flushChangesFile]!
- ] ensure: [self flushChangesFile]!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Monticello-bf.504.mcz

Nicolas Cellier
Ah thanks, very useful.
I noticed the bug, suspected the origin but didn't had time to correct.

Nicolas

Le 19 avril 2012 18:36,  <[hidden email]> a écrit :

> Bert Freudenberg uploaded a new version of Monticello to project The Trunk:
> http://source.squeak.org/trunk/Monticello-bf.504.mcz
>
> ==================== Summary ====================
>
> Name: Monticello-bf.504
> Author: bf
> Time: 19 April 2012, 9:36:55.211 am
> UUID: 5fa1177d-b1ed-4cf0-8628-229f171ca814
> Ancestors: Monticello-eem.503
>
> Fix intermittant load problem. We sometimes would get a 'changes file not found' in the middle of updating, but retrying found it just fine. I finally tracked it down to too many files being open. This change re-uses the same read-only copy during a package load (which additionally might speed up loading a tiny bit).
>
> =============== Diff against Monticello-eem.503 ===============
>
> Item was changed:
>  ----- Method: MCPackageLoader>>basicLoad (in category 'private') -----
>  basicLoad
>        "Load the contents of some package. This is the core loading method
>        in Monticello. Be wary about modifying it unless you understand the details
>        and dependencies of the various entities being modified."
>        | pkgName |
>        errorDefinitions := OrderedCollection new.
>        "Obviously this isn't the package name but we don't have anything else
>        to use here. ChangeSet current name will generally work since a CS is
>        usually installed prior to installation."
>        pkgName := ChangeSet current name.
>
> +       [CurrentReadOnlySourceFiles cacheDuring: [[
> +       "Pass 1: Load everything but the methods,  which are collected in methodAdditions."
> -       [["Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>        additions do: [:ea |
>                ea isMethodDefinition
>                        ifTrue:[methodAdditions add: ea asMethodAddition]
>                        ifFalse:[[ea load]on: Error do: [errorDefinitions add: ea]].
>        ] displayingProgress: 'Reshaping ', pkgName.
>
>        "Try again any delayed definitions"
>        self shouldWarnAboutErrors ifTrue: [self warnAboutErrors].
>        errorDefinitions do: [:ea | ea load]
>                displayingProgress: 'Reloading ', pkgName.
>
>        "Pass 2: We compile new / changed methods"
>        methodAdditions do:[:ea| ea createCompiledMethod]
>                displayingProgress: 'Compiling ', pkgName.
>
>        'Installing ', pkgName displayProgressFrom: 0 to: 2 during:[:bar|
>                "There is no progress *during* installation since a progress bar update
>                will redraw the world and potentially call methods that we're just trying to install."
>                bar value: 1.
>
>                "Pass 3: Install the new / changed methods
>                (this is a separate pass to allow compiler changes to be loaded)"
>                methodAdditions do:[:ea| ea installMethod].
>
>                "Pass 4: Remove the obsolete methods"
>                removals do:[:ea| ea unload].
>        ].
>
>        "Finally, notify observers for the method additions"
>        methodAdditions do: [:each | each notifyObservers]
>                "the message is fake but actually telling people how much time we spend
>                in the notifications is embarrassing so lie instead"
>                displayingProgress: 'Installing ', pkgName.
>
>        additions do: [:ea | ea postloadOver: (self obsoletionFor: ea)]
>                displayingProgress: 'Initializing ', pkgName.
>
>        ] on: InMidstOfFileinNotification do: [:n | n resume: true]
> +       ]] ensure: [self flushChangesFile]!
> -       ] ensure: [self flushChangesFile]!
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Monticello-bf.504.mcz

Bert Freudenberg
I still don't quite understand how too many files could be open. But that's the error I saw at the VM level - and it only happens on Mac and Linux (which share the same file plugin code).

- Bert -

On 19.04.2012, at 10:36, Nicolas Cellier wrote:

> Ah thanks, very useful.
> I noticed the bug, suspected the origin but didn't had time to correct.
>
> Nicolas
>
> Le 19 avril 2012 18:36,  <[hidden email]> a écrit :
>> Bert Freudenberg uploaded a new version of Monticello to project The Trunk:
>> http://source.squeak.org/trunk/Monticello-bf.504.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Monticello-bf.504
>> Author: bf
>> Time: 19 April 2012, 9:36:55.211 am
>> UUID: 5fa1177d-b1ed-4cf0-8628-229f171ca814
>> Ancestors: Monticello-eem.503
>>
>> Fix intermittant load problem. We sometimes would get a 'changes file not found' in the middle of updating, but retrying found it just fine. I finally tracked it down to too many files being open. This change re-uses the same read-only copy during a package load (which additionally might speed up loading a tiny bit).
>>
>> =============== Diff against Monticello-eem.503 ===============
>>
>> Item was changed:
>>  ----- Method: MCPackageLoader>>basicLoad (in category 'private') -----
>>  basicLoad
>>        "Load the contents of some package. This is the core loading method
>>        in Monticello. Be wary about modifying it unless you understand the details
>>        and dependencies of the various entities being modified."
>>        | pkgName |
>>        errorDefinitions := OrderedCollection new.
>>        "Obviously this isn't the package name but we don't have anything else
>>        to use here. ChangeSet current name will generally work since a CS is
>>        usually installed prior to installation."
>>        pkgName := ChangeSet current name.
>>
>> +       [CurrentReadOnlySourceFiles cacheDuring: [[
>> +       "Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>> -       [["Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>>        additions do: [:ea |
>>                ea isMethodDefinition
>>                        ifTrue:[methodAdditions add: ea asMethodAddition]
>>                        ifFalse:[[ea load]on: Error do: [errorDefinitions add: ea]].
>>        ] displayingProgress: 'Reshaping ', pkgName.
>>
>>        "Try again any delayed definitions"
>>        self shouldWarnAboutErrors ifTrue: [self warnAboutErrors].
>>        errorDefinitions do: [:ea | ea load]
>>                displayingProgress: 'Reloading ', pkgName.
>>
>>        "Pass 2: We compile new / changed methods"
>>        methodAdditions do:[:ea| ea createCompiledMethod]
>>                displayingProgress: 'Compiling ', pkgName.
>>
>>        'Installing ', pkgName displayProgressFrom: 0 to: 2 during:[:bar|
>>                "There is no progress *during* installation since a progress bar update
>>                will redraw the world and potentially call methods that we're just trying to install."
>>                bar value: 1.
>>
>>                "Pass 3: Install the new / changed methods
>>                (this is a separate pass to allow compiler changes to be loaded)"
>>                methodAdditions do:[:ea| ea installMethod].
>>
>>                "Pass 4: Remove the obsolete methods"
>>                removals do:[:ea| ea unload].
>>        ].
>>
>>        "Finally, notify observers for the method additions"
>>        methodAdditions do: [:each | each notifyObservers]
>>                "the message is fake but actually telling people how much time we spend
>>                in the notifications is embarrassing so lie instead"
>>                displayingProgress: 'Installing ', pkgName.
>>
>>        additions do: [:ea | ea postloadOver: (self obsoletionFor: ea)]
>>                displayingProgress: 'Initializing ', pkgName.
>>
>>        ] on: InMidstOfFileinNotification do: [:n | n resume: true]
>> +       ]] ensure: [self flushChangesFile]!
>> -       ] ensure: [self flushChangesFile]!
>>
>>
>





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Monticello-bf.504.mcz

Eliot Miranda-2


On Thu, Apr 19, 2012 at 10:46 AM, Bert Freudenberg <[hidden email]> wrote:
I still don't quite understand how too many files could be open. But that's the error I saw at the VM level - and it only happens on Mac and Linux (which share the same file plugin code).

Personally I don't like the CurrentReadOnlySourceFiles cacheDuring: idiom because it forces too many clients to be aware.  Better is to make SourceFiles a self-contained object that maintains writeable and read-only files for sources and changes and have CompiledMethod ChangeRecord et al explicitly use the read-only files as appropriate.  I did this in Newspeak images and it worked well.


- Bert -

On 19.04.2012, at 10:36, Nicolas Cellier wrote:

> Ah thanks, very useful.
> I noticed the bug, suspected the origin but didn't had time to correct.
>
> Nicolas
>
> Le 19 avril 2012 18:36,  <[hidden email]> a écrit :
>> Bert Freudenberg uploaded a new version of Monticello to project The Trunk:
>> http://source.squeak.org/trunk/Monticello-bf.504.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Monticello-bf.504
>> Author: bf
>> Time: 19 April 2012, 9:36:55.211 am
>> UUID: 5fa1177d-b1ed-4cf0-8628-229f171ca814
>> Ancestors: Monticello-eem.503
>>
>> Fix intermittant load problem. We sometimes would get a 'changes file not found' in the middle of updating, but retrying found it just fine. I finally tracked it down to too many files being open. This change re-uses the same read-only copy during a package load (which additionally might speed up loading a tiny bit).
>>
>> =============== Diff against Monticello-eem.503 ===============
>>
>> Item was changed:
>>  ----- Method: MCPackageLoader>>basicLoad (in category 'private') -----
>>  basicLoad
>>        "Load the contents of some package. This is the core loading method
>>        in Monticello. Be wary about modifying it unless you understand the details
>>        and dependencies of the various entities being modified."
>>        | pkgName |
>>        errorDefinitions := OrderedCollection new.
>>        "Obviously this isn't the package name but we don't have anything else
>>        to use here. ChangeSet current name will generally work since a CS is
>>        usually installed prior to installation."
>>        pkgName := ChangeSet current name.
>>
>> +       [CurrentReadOnlySourceFiles cacheDuring: [[
>> +       "Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>> -       [["Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>>        additions do: [:ea |
>>                ea isMethodDefinition
>>                        ifTrue:[methodAdditions add: ea asMethodAddition]
>>                        ifFalse:[[ea load]on: Error do: [errorDefinitions add: ea]].
>>        ] displayingProgress: 'Reshaping ', pkgName.
>>
>>        "Try again any delayed definitions"
>>        self shouldWarnAboutErrors ifTrue: [self warnAboutErrors].
>>        errorDefinitions do: [:ea | ea load]
>>                displayingProgress: 'Reloading ', pkgName.
>>
>>        "Pass 2: We compile new / changed methods"
>>        methodAdditions do:[:ea| ea createCompiledMethod]
>>                displayingProgress: 'Compiling ', pkgName.
>>
>>        'Installing ', pkgName displayProgressFrom: 0 to: 2 during:[:bar|
>>                "There is no progress *during* installation since a progress bar update
>>                will redraw the world and potentially call methods that we're just trying to install."
>>                bar value: 1.
>>
>>                "Pass 3: Install the new / changed methods
>>                (this is a separate pass to allow compiler changes to be loaded)"
>>                methodAdditions do:[:ea| ea installMethod].
>>
>>                "Pass 4: Remove the obsolete methods"
>>                removals do:[:ea| ea unload].
>>        ].
>>
>>        "Finally, notify observers for the method additions"
>>        methodAdditions do: [:each | each notifyObservers]
>>                "the message is fake but actually telling people how much time we spend
>>                in the notifications is embarrassing so lie instead"
>>                displayingProgress: 'Installing ', pkgName.
>>
>>        additions do: [:ea | ea postloadOver: (self obsoletionFor: ea)]
>>                displayingProgress: 'Initializing ', pkgName.
>>
>>        ] on: InMidstOfFileinNotification do: [:n | n resume: true]
>> +       ]] ensure: [self flushChangesFile]!
>> -       ] ensure: [self flushChangesFile]!
>>
>>
>








--
best,
Eliot



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Monticello-bf.504.mcz

Levente Uzonyi-2
On Thu, 19 Apr 2012, Eliot Miranda wrote:

>
>
> On Thu, Apr 19, 2012 at 10:46 AM, Bert Freudenberg <[hidden email]> wrote:
>       I still don't quite understand how too many files could be open. But that's the error I saw at the VM level - and it only happens on Mac and Linux (which share the same file plugin code).
>
>
> Personally I don't like the CurrentReadOnlySourceFiles cacheDuring: idiom because it forces too many clients to be aware.  Better is to make SourceFiles a self-contained object that maintains writeable and read-only files for sources
> and changes and have CompiledMethod ChangeRecord et al explicitly use the read-only files as appropriate.  I did this in Newspeak images and it worked well.

It's not perfect, but since FileStreams are not thread safe in the image,
we need at least one read-only instance per process. I wanted to rewrite
the caching to use process local variables after I added them to the
Trunk, but didn't have time for that, so 4.3 came out with the current
solution.


Levente

>
>
>       - Bert -
>
>       On 19.04.2012, at 10:36, Nicolas Cellier wrote:
>
>       > Ah thanks, very useful.
>       > I noticed the bug, suspected the origin but didn't had time to correct.
>       >
>       > Nicolas
>       >
>       > Le 19 avril 2012 18:36,  <[hidden email]> a écrit :
>       >> Bert Freudenberg uploaded a new version of Monticello to project The Trunk:
>       >> http://source.squeak.org/trunk/Monticello-bf.504.mcz
>       >>
>       >> ==================== Summary ====================
>       >>
>       >> Name: Monticello-bf.504
>       >> Author: bf
>       >> Time: 19 April 2012, 9:36:55.211 am
>       >> UUID: 5fa1177d-b1ed-4cf0-8628-229f171ca814
>       >> Ancestors: Monticello-eem.503
>       >>
>       >> Fix intermittant load problem. We sometimes would get a 'changes file not found' in the middle of updating, but retrying found it just fine. I finally tracked it down to too many files being open. This change re-uses
>       the same read-only copy during a package load (which additionally might speed up loading a tiny bit).
>       >>
>       >> =============== Diff against Monticello-eem.503 ===============
>       >>
>       >> Item was changed:
>       >>  ----- Method: MCPackageLoader>>basicLoad (in category 'private') -----
>       >>  basicLoad
>       >>        "Load the contents of some package. This is the core loading method
>       >>        in Monticello. Be wary about modifying it unless you understand the details
>       >>        and dependencies of the various entities being modified."
>       >>        | pkgName |
>       >>        errorDefinitions := OrderedCollection new.
>       >>        "Obviously this isn't the package name but we don't have anything else
>       >>        to use here. ChangeSet current name will generally work since a CS is
>       >>        usually installed prior to installation."
>       >>        pkgName := ChangeSet current name.
>       >>
>       >> +       [CurrentReadOnlySourceFiles cacheDuring: [[
>       >> +       "Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>       >> -       [["Pass 1: Load everything but the methods,  which are collected in methodAdditions."
>       >>        additions do: [:ea |
>       >>                ea isMethodDefinition
>       >>                        ifTrue:[methodAdditions add: ea asMethodAddition]
>       >>                        ifFalse:[[ea load]on: Error do: [errorDefinitions add: ea]].
>       >>        ] displayingProgress: 'Reshaping ', pkgName.
>       >>
>       >>        "Try again any delayed definitions"
>       >>        self shouldWarnAboutErrors ifTrue: [self warnAboutErrors].
>       >>        errorDefinitions do: [:ea | ea load]
>       >>                displayingProgress: 'Reloading ', pkgName.
>       >>
>       >>        "Pass 2: We compile new / changed methods"
>       >>        methodAdditions do:[:ea| ea createCompiledMethod]
>       >>                displayingProgress: 'Compiling ', pkgName.
>       >>
>       >>        'Installing ', pkgName displayProgressFrom: 0 to: 2 during:[:bar|
>       >>                "There is no progress *during* installation since a progress bar update
>       >>                will redraw the world and potentially call methods that we're just trying to install."
>       >>                bar value: 1.
>       >>
>       >>                "Pass 3: Install the new / changed methods
>       >>                (this is a separate pass to allow compiler changes to be loaded)"
>       >>                methodAdditions do:[:ea| ea installMethod].
>       >>
>       >>                "Pass 4: Remove the obsolete methods"
>       >>                removals do:[:ea| ea unload].
>       >>        ].
>       >>
>       >>        "Finally, notify observers for the method additions"
>       >>        methodAdditions do: [:each | each notifyObservers]
>       >>                "the message is fake but actually telling people how much time we spend
>       >>                in the notifications is embarrassing so lie instead"
>       >>                displayingProgress: 'Installing ', pkgName.
>       >>
>       >>        additions do: [:ea | ea postloadOver: (self obsoletionFor: ea)]
>       >>                displayingProgress: 'Initializing ', pkgName.
>       >>
>       >>        ] on: InMidstOfFileinNotification do: [:n | n resume: true]
>       >> +       ]] ensure: [self flushChangesFile]!
>       >> -       ] ensure: [self flushChangesFile]!
>       >>
>       >>
>       >
>
>
>
>
>
>
>
>
> --
> best,Eliot
>
>
>