CommandHistory class #forgetAllGrabCommandsFrom:

On 11/5/07, Rob Withers <[hidden email]> wrote:
> Tom, the original loop that was looping forever was the method
> #forgetAllGrabCommandsFrom:

>     | object |
>     object _ starter.
>     [
>         [0 == object] whileFalse: [
>             object isMorph ifTrue: [object removeProperty:
> #undoGrabCommand].
>             object _ object nextObject].
>         ] ifError: [:err :rcvr | "object is obsolete"
>             self forgetAllGrabCommandsFrom: object nextObject].

That's a botch, every way I look at it. There's no need of the
parameter, since everybody wants to start at the beginning. Somebody
didn't know they could send #allSubInstancesDo: to Morph, which would
do this much more efficiently and without the possible #become bug.
Somebody else(?) added the error trapping to deal with obsolete
objects, but they could have tested rcvr isObsolete and not risked
hiding a different error in the process.

Is there any justification for the way this is done currently in
CommandHistory class #forgetAllGrabCommandsFrom:, instead of something
like this?

  CommandHistory class>>forgetAllGrabCommands
    Morph allSubInstancesDo: [:m |
      m removeProperty: #undoGrabCommand ].

Cheers!

--Tom Phoenix

----- Original Message -----
From: "Tom Phoenix" <[hidden email]>

>
>  CommandHistory class>>forgetAllGrabCommands
>    Morph allSubInstancesDo: [:m |
>      m removeProperty: #undoGrabCommand ].

Tom, this certainly looks better, but per John, it would be slower than the
loop with #nextObject.  This runs on every image startUp.

Plus I want to find out why my change caused such a disaster.

Cheers,
Rob

Rob Withers wrote:
Check the allocation patterns. The following should do the trick:

   marker := Object new. "end marker"
   [marker == obj] whileFalse:[
     "... do whatever was here before ..."
     obj := obj nextObject.
   ].

   "Now we're running into the objects that were generated during the
above loop. Create a second marker to guard against runaway and see what
objects were created during the above loop"

   marker := Object new. "end marker"
   tally := OrderedCollection new: 10000.
   [marker == obj] whileFalse:[
     tally add: obj.
     obj := obj nextObject.
   ].

Cheers,
   - Andreas

That's nifty, Andreas, and the results are surprising.  There are about 600
objects allocated while in the first loop.  It fluctuates as low as 550 and
as high as 650 or so.  Most of them are the BlockContexts created after the
ifTrue:, while some are MethodContexts of the ifTrue: and then some others
relating to the removeProperty: msg.

Here was the most common, by far.
[] in UndefinedObject>>DoIt {[obj removeProperty: #undoGrabCommand]}

from this line of code in the work section of your first loop:
 obj isMorph ifTrue: [obj removeProperty: #undoGrabCommand].

Why only 650 and why does this cause infinite ingress of new allocations?
So I extended your loop-allocation pattern to have 6 intermediate loops,
then check for allocations on the last intermediate loop.

I have more BlockContext, 7 to 60 of them, it fluctuates:
[] in UndefinedObject>>DoIt {[obj removeProperty: #undoGrabCommand]}

In a way it makes sense.  Now that ifTrue: is not inlined, then it is doing
an explicit blockCopy: to create this block.  BlockContexts are not recycled
so they are always new objects.  Whether the receiver of the ifTrue: is true
or false, this block will be created.  Infinite loop.  This code depends on
ifTrue: being inline.

The fluctuations are interesting and may be some evidence that GCs are
occurring.

Cheers,
Rob

----- Original Message -----
From: "Andreas Raab" <[hidden email]>
To: "The general-purpose Squeak developers list"
<[hidden email]>
Sent: Monday, November 05, 2007 4:37 PM
Subject: Re: CommandHistory class #forgetAllGrabCommandsFrom: