[Etoys] Performance improvements form 3.9 (backported) #3

[Scott Wallace]

Here are a couple of issues arising from these mostly very welcome  
changes:


(1)  I happened to notice that "Color fromUser" had become broken in  
OLPC after loading the performance improvements.  I eventually  
tracked its cause down to update 1074ShortCutNew-md.  Because of the  
change to Rectangle class origin:corner: in that update, #initialize  
is no longer sent to a new Quadrangle created via  
Quadrangle>>origin:corner, with the result that Quadrangles created  
with this idiom are incompletely initialized, with nil in their  
borderWidth, borderColor, and insideColor instance variables; this is  
what broke "Color fromUser".

(BTW CharacterBlock, another Rectangle subclass which has its own  
inst var additions, might also have been broken by the ShortCutNew  
change -- someone ought to check.)

When I evaluated "Color fromUser" in a 3.9 image, it didn't cause the  
error I saw in olpc.  Investigating further, I found that this was  
because in 3.9 a custom @origin:corner: had been added to Quadrangle  
class-side.  But that addition did not make it into the bundle that  
got sent to us.  [BTW it appears that while 3.9 has the fix for  
Quadrangle>>origin:corner:, it still has the bug for  
Quadrangle>>origin:extent:]

I've pushed an update to olpc that fixes the two bugs I know of that  
appeared in Quadrangle as a consequence of update 1074, so I mention  
the issue here only as a cautionary tale.  Partial backports can be  
dangerous, sometimes in quite obscure ways...


(2)  I have reservations about the SubMorphCopy change-set  
(1075SubMorphCopy-md).

A well-known feature of Morphic's api was that #submorphs returned a  
shallow copy, rather than handing out the submorphs object itself.   
The SubMorphCopy update rescinds Morphic's long-standing (> 10 years)  
promise that #submorphs would return a new collection.

Many of us have had experience with bugs caused by iterating over a  
collection when the code run within the iteration might change the  
actual collection being iterated over.  Thus, for example, as a  
safety measure, we often use the idiom "foo copy do:", as opposed to  
"foo do:", in situations where the actual contents of the foo  
collection might get changed by the do-block, thus compromising the  
integrity of the iteration itself.  However, knowing that #submorphs  
always returned a *copy*, it has (until now) always been reasonable  
-- indeed optimal -- to omit a redundant #copy call in such  
circumstances.

Now it's *possible* that no methods linger anywhere in the image, nor  
in any package likely to be loaded by any user of an olpc image,  
which count on the promise that #submorphs would return a copy.  But  
the only experience in the field with the non-copying #submorphs  
implementation has been in 3.9, and the total amount of etoy activity  
by people using 3.9 is approximately zero, whereas on the olpc nearly  
all Squeak activity will relate to etoys.

The likelihood that something actually got broken by switching to the  
non-copying #submorphs may be small, but IMO it's decidedly nonzero;  
and my concern is that if there *are* some breakages caused by this  
change, they might be extremely hard to track down.

So I wonder about the advisability of making a fundamental semantic  
change to a fundamental Morphic protocol at this stage, even if it  
appears that they've "gotten away with it" in 3.9...


Cheers,

   -- Scott

On Oct 16, 2006, at 12:20 PM, Yoshiki Ohshima wrote:

>   Marcus,
>
>>>> Change Set:		SubMorphCopy-md
>>>>   This one was considered "it seems to be ok.  We don't find any  
>>>> code
>>> that changes the result".  I don't remember the further discussion,
>>> but was there some analysis, or it is just seems to be ok as it is
>>> long time in 3.9 line images?
>>
>> It's in 3.9 for some time now, yes. But it should not be really  
>> performance
>> critical, so you might not want to add it.
>
>   It does make difference, though.  I like it in the image.  There
> seems to be small small chance that some area breaks, but it should be
> something we can fix later.