The Inbox: System-edc.512.mcz

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

The Inbox: System-edc.512.mcz

commits-2
Edgar J. De Cleene uploaded a new version of System to project The Inbox:
http://source.squeak.org/inbox/System-edc.512.mcz

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

Name: System-edc.512
Author: edc
Time: 21 January 2013, 11:05:08.17 am
UUID: d7a321ce-cf99-4b7d-9c9f-12e0470672a7
Ancestors: System-fbs.511

When you update from server , the resuting image is not cleaned and bigger as should be.

=============== Diff against System-fbs.511 ===============

Item was changed:
  ----- Method: Utilities class>>updateFromServer (in category 'fetching updates') -----
  updateFromServer
  "Update the image by loading all pending updates from the server."
  | config |
  "Flush all caches. If a previous download failed this is often helpful"
  MCFileBasedRepository flushAllCaches.
  config := MCMcmUpdater updateFromDefaultRepository.
  config ifNil: [^self inform: 'Unable to retrieve updates from remote repository.' translated].
  self setSystemVersionFromConfig: config.
  self inform: ('Update completed.
+ Current update number: ' translated, SystemVersion current highestUpdate).
+ self updateFromServerCleanup.!
- Current update number: ' translated, SystemVersion current highestUpdate).!

Item was added:
+ ----- Method: Utilities class>>updateFromServerCleanup (in category 'fetching updates') -----
+ updateFromServerCleanup
+ MCFileBasedRepository flushAllCaches.
+ MCDefinition clearInstances.
+ 3 timesRepeat: [ Smalltalk garbageCollect].
+ !


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Bert Freudenberg
On 21.01.2013, at 14:05, [hidden email] wrote:

> 3 timesRepeat: [ Smalltalk garbageCollect].

Doing this 3 times is superstitious. It won't clean up more than doing it once.

IMHO an explicit GC is not necessary at all - what purpose does it serve?

- Bert -


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Nicolas Cellier
Ah ah, I wanted to ask if 3 timesRepeat garbageCollect was a kind of
magical thinking or a known limitation of our collector :)

I noticed several usages of such snippet, in ReleaseBuilder and
ScriptLoader and WeakSetInspectorTest, but oh, the last has my
initials stamped :(

I also wonder what are the effects of flushing the caches...

As I understand it, MCFileBasedRepository flushAllCaches will ignore
the copy already in package-cache and force a re-load from network. It
is already present above in code, so let's consider the effect is
benign.
(Personnaly I would only trigger a reload action in exception
handling, because if network is unreliable, there is a high risk to
replace a finally valid load by an invalid one).

I have no idea of what (MCDefinition clearInstances) does, all I see
is that 'instances' variable is weak, so I presume it is supposed to
auto-clean. While chasing the pointers of MCDefinition
allSubInstances, I noticed a lot of MC related scories in Object's
DependentFields.
So we can consider this cache flushing as a workaround, but should
better chase the buggish dependents...

Nicolas

2013/1/21 Bert Freudenberg <[hidden email]>:

> On 21.01.2013, at 14:05, [hidden email] wrote:
>
>> 3 timesRepeat: [ Smalltalk garbageCollect].
>
> Doing this 3 times is superstitious. It won't clean up more than doing it once.
>
> IMHO an explicit GC is not necessary at all - what purpose does it serve?
>
> - Bert -
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Chris Muller-3
In reply to this post by commits-2
-10.  Updating the image is done under many use-cases, not just
building an image.  There are many cases where you would not want this
done.  Cleaning the image must be its own independent operation
independent of other operations, especially the ones for _building_
the image.

On Mon, Jan 21, 2013 at 8:05 AM,  <[hidden email]> wrote:

> Edgar J. De Cleene uploaded a new version of System to project The Inbox:
> http://source.squeak.org/inbox/System-edc.512.mcz
>
> ==================== Summary ====================
>
> Name: System-edc.512
> Author: edc
> Time: 21 January 2013, 11:05:08.17 am
> UUID: d7a321ce-cf99-4b7d-9c9f-12e0470672a7
> Ancestors: System-fbs.511
>
> When you update from server , the resuting image is not cleaned and bigger as should be.
>
> =============== Diff against System-fbs.511 ===============
>
> Item was changed:
>   ----- Method: Utilities class>>updateFromServer (in category 'fetching updates') -----
>   updateFromServer
>         "Update the image by loading all pending updates from the server."
>         | config |
>         "Flush all caches. If a previous download failed this is often helpful"
>         MCFileBasedRepository flushAllCaches.
>         config := MCMcmUpdater updateFromDefaultRepository.
>         config ifNil: [^self inform: 'Unable to retrieve updates from remote repository.' translated].
>         self setSystemVersionFromConfig: config.
>         self inform: ('Update completed.
> + Current update number: ' translated, SystemVersion current highestUpdate).
> + self updateFromServerCleanup.!
> - Current update number: ' translated, SystemVersion current highestUpdate).!
>
> Item was added:
> + ----- Method: Utilities class>>updateFromServerCleanup (in category 'fetching updates') -----
> + updateFromServerCleanup
> +       MCFileBasedRepository flushAllCaches.
> + MCDefinition clearInstances.
> + 3 timesRepeat: [ Smalltalk garbageCollect].
> + !
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Nicolas Cellier
That was my first thought, ask Edgar whether the intention was for
image production.
And ask about the impact of such change onto every day update.
But it seems it has the merit of uncovring some ghost dependents bug.

2013/1/21 Chris Muller <[hidden email]>:

> -10.  Updating the image is done under many use-cases, not just
> building an image.  There are many cases where you would not want this
> done.  Cleaning the image must be its own independent operation
> independent of other operations, especially the ones for _building_
> the image.
>
> On Mon, Jan 21, 2013 at 8:05 AM,  <[hidden email]> wrote:
>> Edgar J. De Cleene uploaded a new version of System to project The Inbox:
>> http://source.squeak.org/inbox/System-edc.512.mcz
>>
>> ==================== Summary ====================
>>
>> Name: System-edc.512
>> Author: edc
>> Time: 21 January 2013, 11:05:08.17 am
>> UUID: d7a321ce-cf99-4b7d-9c9f-12e0470672a7
>> Ancestors: System-fbs.511
>>
>> When you update from server , the resuting image is not cleaned and bigger as should be.
>>
>> =============== Diff against System-fbs.511 ===============
>>
>> Item was changed:
>>   ----- Method: Utilities class>>updateFromServer (in category 'fetching updates') -----
>>   updateFromServer
>>         "Update the image by loading all pending updates from the server."
>>         | config |
>>         "Flush all caches. If a previous download failed this is often helpful"
>>         MCFileBasedRepository flushAllCaches.
>>         config := MCMcmUpdater updateFromDefaultRepository.
>>         config ifNil: [^self inform: 'Unable to retrieve updates from remote repository.' translated].
>>         self setSystemVersionFromConfig: config.
>>         self inform: ('Update completed.
>> + Current update number: ' translated, SystemVersion current highestUpdate).
>> + self updateFromServerCleanup.!
>> - Current update number: ' translated, SystemVersion current highestUpdate).!
>>
>> Item was added:
>> + ----- Method: Utilities class>>updateFromServerCleanup (in category 'fetching updates') -----
>> + updateFromServerCleanup
>> +       MCFileBasedRepository flushAllCaches.
>> + MCDefinition clearInstances.
>> + 3 timesRepeat: [ Smalltalk garbageCollect].
>> + !
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Hannes Hirzel
In reply to this post by Chris Muller-3
On 1/21/13, Chris Muller <[hidden email]> wrote:
> -10.  Updating the image is done under many use-cases, not just
> building an image.  There are many cases where you would not want this
> done.


Such as for example?


Cleaning the image must be its own independent operation
> independent of other operations, especially the ones for _building_
> the image.


The fact that Edgar points out is that the code snippet makes the
image smaller. This is indeed the case.

A fresh build from the trunk as of today goes down from
http://www.squeakci.org/job/SqueakTrunk/

16.7 MB to 15.287 MB


The number of times a garbage collection is made (1 or 3) indeed does
not have a visible effect.

It might be that this was necessary in earlier versions.



--Hannes



> On Mon, Jan 21, 2013 at 8:05 AM,  <[hidden email]> wrote:
>> Edgar J. De Cleene uploaded a new version of System to project The Inbox:
>> http://source.squeak.org/inbox/System-edc.512.mcz
>>
>> ==================== Summary ====================
>>
>> Name: System-edc.512
>> Author: edc
>> Time: 21 January 2013, 11:05:08.17 am
>> UUID: d7a321ce-cf99-4b7d-9c9f-12e0470672a7
>> Ancestors: System-fbs.511
>>
>> When you update from server , the resuting image is not cleaned and bigger
>> as should be.
>>
>> =============== Diff against System-fbs.511 ===============
>>
>> Item was changed:
>>   ----- Method: Utilities class>>updateFromServer (in category 'fetching
>> updates') -----
>>   updateFromServer
>>         "Update the image by loading all pending updates from the
>> server."
>>         | config |
>>         "Flush all caches. If a previous download failed this is often
>> helpful"
>>         MCFileBasedRepository flushAllCaches.
>>         config := MCMcmUpdater updateFromDefaultRepository.
>>         config ifNil: [^self inform: 'Unable to retrieve updates from
>> remote repository.' translated].
>>         self setSystemVersionFromConfig: config.
>>         self inform: ('Update completed.
>> + Current update number: ' translated, SystemVersion current
>> highestUpdate).
>> + self updateFromServerCleanup.!
>> - Current update number: ' translated, SystemVersion current
>> highestUpdate).!
>>
>> Item was added:
>> + ----- Method: Utilities class>>updateFromServerCleanup (in category
>> 'fetching updates') -----
>> + updateFromServerCleanup
>> +       MCFileBasedRepository flushAllCaches.
>> + MCDefinition clearInstances.
>> + 3 timesRepeat: [ Smalltalk garbageCollect].
>> + !
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Hannes Hirzel
In reply to this post by Nicolas Cellier
On 1/21/13, Nicolas Cellier <[hidden email]> wrote:
> That was my first thought, ask Edgar whether the intention was for
> image production.
> And ask about the impact of such change onto every day update.
> But it seems it has the merit of uncovring some ghost dependents bug.

+1

>
> 2013/1/21 Chris Muller <[hidden email]>:
>> -10.  Updating the image is done under many use-cases, not just
>> building an image.  There are many cases where you would not want this
>> done.  Cleaning the image must be its own independent operation
>> independent of other operations, especially the ones for _building_
>> the image.
>>
>> On Mon, Jan 21, 2013 at 8:05 AM,  <[hidden email]> wrote:
>>> Edgar J. De Cleene uploaded a new version of System to project The
>>> Inbox:
>>> http://source.squeak.org/inbox/System-edc.512.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: System-edc.512
>>> Author: edc
>>> Time: 21 January 2013, 11:05:08.17 am
>>> UUID: d7a321ce-cf99-4b7d-9c9f-12e0470672a7
>>> Ancestors: System-fbs.511
>>>
>>> When you update from server , the resuting image is not cleaned and
>>> bigger as should be.
>>>
>>> =============== Diff against System-fbs.511 ===============
>>>
>>> Item was changed:
>>>   ----- Method: Utilities class>>updateFromServer (in category 'fetching
>>> updates') -----
>>>   updateFromServer
>>>         "Update the image by loading all pending updates from the
>>> server."
>>>         | config |
>>>         "Flush all caches. If a previous download failed this is often
>>> helpful"
>>>         MCFileBasedRepository flushAllCaches.
>>>         config := MCMcmUpdater updateFromDefaultRepository.
>>>         config ifNil: [^self inform: 'Unable to retrieve updates from
>>> remote repository.' translated].
>>>         self setSystemVersionFromConfig: config.
>>>         self inform: ('Update completed.
>>> + Current update number: ' translated, SystemVersion current
>>> highestUpdate).
>>> + self updateFromServerCleanup.!
>>> - Current update number: ' translated, SystemVersion current
>>> highestUpdate).!
>>>
>>> Item was added:
>>> + ----- Method: Utilities class>>updateFromServerCleanup (in category
>>> 'fetching updates') -----
>>> + updateFromServerCleanup
>>> +       MCFileBasedRepository flushAllCaches.
>>> + MCDefinition clearInstances.
>>> + 3 timesRepeat: [ Smalltalk garbageCollect].
>>> + !
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Edgar De Cleene
In reply to this post by Nicolas Cellier
> 3 timesRepeat: [ Smalltalk garbageCollect].

Doing this 3 times is superstitious. It won't clean up more than doing it
once.

IMHO an explicit GC is not necessary at all - what purpose does it serve?

- Bert -

About the 3 timesRepeat: [ Smalltalk garbageCollect], I see many times from
the past, but if Smalltalk garbageCollect is enough, is good to know.

Anyway , none of my cats is black and seeking for a horseshoe with seven
holes just in case.

On 1/21/13 6:12 PM, "Nicolas Cellier" <[hidden email]>
wrote:

> That was my first thought, ask Edgar whether the intention was for
> image production.
> And ask about the impact of such change onto every day update.
> But it seems it has the merit of uncovring some ghost dependents bug.

Well , I come from age when you just hit update image and not download a new
one each time.

If you don't clean the image, ends with 17 Mb instead 15 Mb.
I know disk space is cheaper this days...

But do not think this hurt any, maybe if the user user choose wish clean or
not is satisfactory to those who disagree ?

Edgar



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Frank Shearar-3
On 21 January 2013 22:13, Edgar J. De Cleene <[hidden email]> wrote:

>> 3 timesRepeat: [ Smalltalk garbageCollect].
>
> Doing this 3 times is superstitious. It won't clean up more than doing it
> once.
>
> IMHO an explicit GC is not necessary at all - what purpose does it serve?
>
> - Bert -
>
> About the 3 timesRepeat: [ Smalltalk garbageCollect], I see many times from
> the past, but if Smalltalk garbageCollect is enough, is good to know.
>
> Anyway , none of my cats is black and seeking for a horseshoe with seven
> holes just in case.
>
> On 1/21/13 6:12 PM, "Nicolas Cellier" <[hidden email]>
> wrote:
>
>> That was my first thought, ask Edgar whether the intention was for
>> image production.
>> And ask about the impact of such change onto every day update.
>> But it seems it has the merit of uncovring some ghost dependents bug.
>
> Well , I come from age when you just hit update image and not download a new
> one each time.
>
> If you don't clean the image, ends with 17 Mb instead 15 Mb.
> I know disk space is cheaper this days...
>
> But do not think this hurt any, maybe if the user user choose wish clean or
> not is satisfactory to those who disagree ?

As long there's a way to update without requiring a user, I'm happy either way.

frank

> Edgar
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Bert Freudenberg
In reply to this post by Nicolas Cellier

On 21.01.2013, at 12:44, Nicolas Cellier <[hidden email]> wrote:

> Ah ah, I wanted to ask if 3 timesRepeat garbageCollect was a kind of
> magical thinking or a known limitation of our collector :)
>
> I noticed several usages of such snippet, in ReleaseBuilder and
> ScriptLoader and WeakSetInspectorTest, but oh, the last has my
> initials stamped :(
>
> I also wonder what are the effects of flushing the caches...
>
> As I understand it, MCFileBasedRepository flushAllCaches will ignore
> the copy already in package-cache and force a re-load from network. It
> is already present above in code, so let's consider the effect is
> benign.
> (Personnaly I would only trigger a reload action in exception
> handling, because if network is unreliable, there is a high risk to
> replace a finally valid load by an invalid one).
>
> I have no idea of what (MCDefinition clearInstances) does, all I see
> is that 'instances' variable is weak, so I presume it is supposed to
> auto-clean. While chasing the pointers of MCDefinition
> allSubInstances, I noticed a lot of MC related scories in Object's
> DependentFields.
> So we can consider this cache flushing as a workaround, but should
> better chase the buggish dependents...
>
> Nicolas

Oh, I didn't even notice the clearInstances. That's definitely not needed as they are weak, as you say.

- Bert -

>
> 2013/1/21 Bert Freudenberg <[hidden email]>:
>> On 21.01.2013, at 14:05, [hidden email] wrote:
>>
>>> 3 timesRepeat: [ Smalltalk garbageCollect].
>>
>> Doing this 3 times is superstitious. It won't clean up more than doing it once.
>>
>> IMHO an explicit GC is not necessary at all - what purpose does it serve?
>>
>> - Bert -
>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Bob Arning-2
In reply to this post by Bert Freudenberg
Perhaps in neat and tidy situations, but I invite your comments on

test1
"
ThreeGCTest test1
"
    | object registry other say |
   
    Smalltalk garbageCollect.
    Transcript cr.
    say _ [ :str |
        Transcript show: Smalltalk garbageCollect asStringWithCommas,' ',str; cr
    ].
    say value: 'start'.
    registry := WeakRegistry new.
    object := Object new.
    other _ String new: 1024*102*16.
    registry add: object executor: (ObjectFinalizer
        receiver: [other _ nil]
        selector: #value).
    say value: 'created'.
    object := nil.
    say value: 'gc1'.
    say value: 'gc2'.
    say value: 'gc3'.

which produces:

505,963,044 start
504,291,464 created
504,291,360 gc1
505,962,536 gc2
505,962,428 gc3

Cheers,
Bob

On 1/21/13 3:28 PM, Bert Freudenberg wrote:
On 21.01.2013, at 14:05, [hidden email] wrote:

3 timesRepeat: [ Smalltalk garbageCollect].
Doing this 3 times is superstitious. It won't clean up more than doing it once.

IMHO an explicit GC is not necessary at all - what purpose does it serve?

- Bert -






Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Bert Freudenberg
On 21.01.2013, at 15:05, Bob Arning <[hidden email]> wrote:

Perhaps in neat and tidy situations, but I invite your comments on

The situation at hand was having to do with MCDefinitions in a WeakSet. Since MCDefinitions don't have finalizers, it won't make any difference. Of course it's possible to construct a chain of finalizers where even 3 GCs would not be enough.

- Bert -



test1
"
ThreeGCTest test1
"
    | object registry other say |
   
    Smalltalk garbageCollect.
    Transcript cr.
    say _ [ :str |
        Transcript show: Smalltalk garbageCollect asStringWithCommas,' ',str; cr
    ].
    say value: 'start'.
    registry := WeakRegistry new.
    object := Object new.
    other _ String new: 1024*102*16.
    registry add: object executor: (ObjectFinalizer
        receiver: [other _ nil]
        selector: #value).
    say value: 'created'.
    object := nil.
    say value: 'gc1'.
    say value: 'gc2'.
    say value: 'gc3'.

which produces:

505,963,044 start
504,291,464 created
504,291,360 gc1
505,962,536 gc2
505,962,428 gc3

Cheers,
Bob

On 1/21/13 3:28 PM, Bert Freudenberg wrote:
On 21.01.2013, at 14:05, [hidden email] wrote:

3 timesRepeat: [ Smalltalk garbageCollect].
Doing this 3 times is superstitious. It won't clean up more than doing it once.

IMHO an explicit GC is not necessary at all - what purpose does it serve?

- Bert -








Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Chris Muller-4
In reply to this post by Hannes Hirzel
> The fact that Edgar points out is that the code snippet makes the
> image smaller. This is indeed the case.
>
> A fresh build from the trunk as of today goes down from
> http://www.squeakci.org/job/SqueakTrunk/
>
> 16.7 MB to 15.287 MB

I'm just sayin' let the ci jobs produce clean images, not
#updateFromServer please.

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-edc.512.mcz

Nicolas Cellier
In reply to this post by Bert Freudenberg
2013/1/22 Bert Freudenberg <[hidden email]>:

>
> On 21.01.2013, at 12:44, Nicolas Cellier <[hidden email]> wrote:
>
>> Ah ah, I wanted to ask if 3 timesRepeat garbageCollect was a kind of
>> magical thinking or a known limitation of our collector :)
>>
>> I noticed several usages of such snippet, in ReleaseBuilder and
>> ScriptLoader and WeakSetInspectorTest, but oh, the last has my
>> initials stamped :(
>>
>> I also wonder what are the effects of flushing the caches...
>>
>> As I understand it, MCFileBasedRepository flushAllCaches will ignore
>> the copy already in package-cache and force a re-load from network. It
>> is already present above in code, so let's consider the effect is
>> benign.
>> (Personnaly I would only trigger a reload action in exception
>> handling, because if network is unreliable, there is a high risk to
>> replace a finally valid load by an invalid one).
>>
>> I have no idea of what (MCDefinition clearInstances) does, all I see
>> is that 'instances' variable is weak, so I presume it is supposed to
>> auto-clean. While chasing the pointers of MCDefinition
>> allSubInstances, I noticed a lot of MC related scories in Object's
>> DependentFields.
>> So we can consider this cache flushing as a workaround, but should
>> better chase the buggish dependents...
>>
>> Nicolas
>
> Oh, I didn't even notice the clearInstances. That's definitely not needed as they are weak, as you say.
>
> - Bert -
>

Even worse:

As I understand it, the purpose of MCDefinition cache is to avoid
having multiple copies of same MCDefinition lying around the image.
So clearing the cache could at worse increase image size by creating
another copy of the same MCDefinition next time one is created.

Unfortunately there is also this problem of MCTool ghosts haunting
Object's DependentFields which let us believe that it's a good idea to
clear the cache...

>>
>> 2013/1/21 Bert Freudenberg <[hidden email]>:
>>> On 21.01.2013, at 14:05, [hidden email] wrote:
>>>
>>>> 3 timesRepeat: [ Smalltalk garbageCollect].
>>>
>>> Doing this 3 times is superstitious. It won't clean up more than doing it once.
>>>
>>> IMHO an explicit GC is not necessary at all - what purpose does it serve?
>>>
>>> - Bert -
>>>
>>>
>>
>
>