Hi Both,

    the adoptInstance: code fails on V3 because of compact classes.  One cannot change a 1-word header instance if a compact class into a 1-word header of a non-compact class; there is no room in a 1-word header for the 32-bit non/compact class reference.

David, fur give me for broaching what may be an emotionally painful topic.  I have no desire to cause you pain, but IMO this is important.

We should /NOT/ hamstring code for either Spur or V3 with needless and expensive compatibility.  David, you're going to have to accept that the two will diverge.  Instead, try and think of a mechanism that supports tracking the differences instead of trying to keep the code bases the same.  IMO effort put into keeping the two the same is effort wasted.  Instead one could put effort into a Spur interpreter VM, or tools for automatic patching of Monticello packages with Spur<->V3 differences.

Bert and Yom and others just put significant effort into etoys on Squeak 5.1 (which is Spur).  Bert wrote a V3 format ImageSegment loader entirely in Smalltalk, allowing Spur to load old etoys projects saved on V3.  This is effort well spent.  It rescues old code  and allows it to live again and solves an important compatibility problem (and indeed a Spur format loader could be written to allow V3 to load ImageSegments saved on Spur).

In a community with extremely limited resources it is incumbent upon those of us who can to find constructive directions for our efforts if we, as a community, are to make progress.

Eliot
_,,,^..^,,,_ (phone)

> On Sep 5, 2016, at 2:12 AM, David T. Lewis <lewis at mail.msen.com> wrote:
> 
> Hi Levente,
> 
> Thanks for looking at this.
> 
>> On Sun, Sep 04, 2016 at 09:56:52PM +0200, Levente Uzonyi wrote:
>> Error handling is slow. It creates many additional objects (blocks, 
>> contexts, exceptions). What #adoptInstance: actually saves here is the 
>> reallocation of a 2k buffer. So I suppose the error handler is slower 
>> than creating a new collection, but this time it's slower for both Spur 
>> and V3.
>> 
>> The real problem is that it seems #adoptInstance: doesn't work in V3. Why 
>> is that?
>> 
>> Levente
> 
> On V3 with either Cog or Interpreter VM this works:
> 
>  ba := ByteArray new: 100.
>  ByteString adoptInstance: ba.
>  ba class ==> ByteString
> 
> But the reverse does not work:
> 
>  bs := ByteString new: 100.
>  ByteArray adoptInstance: bs. ==> Error
> 
> 
> 
>> P.S.: I wrote a small script to see the performance difference and ran it 
>> on Spur. It shows that my assumption above was incorrect. Still, the 
>> overhead is rather high, so there's an alternative workaround at the 
>> bottom, which should be quicker on both V3 and Spur than using exceptions. 
>> But I still think that #adoptInstance: should work on V3, too.
> 
> 
> I tried some more tests, using file streams to hopefully make the results
> more realistic. I was completely surprised by the results, and I cannot say
> that there is any best approach. I guess I would summarize:
> 
> - The adoptInstance: optimization provides a significant improvement
>  for Spur.
> - For Spur, the #isRunningSpur test works much better than exception
>  handling. But Spur is so fast that it really does not matter what
>  you do as long as you use the adoptInstance: optimization.
> - For V3 images with Cog, the exception handler approach works better
>  than the #isRunningSpur test.
> - For V3 images on interpreter VM, performance is slower and it it does
>  not matter what approach is used.
> 
> Conclusion: Either the exception handler approach or the #isRunningSpur
> test will work reasonably well. Personally I do not like using exception
> handlers to control program flow, so I would prefer your (Levente's)
> approach of explicitly testing for #isRunningSpur (or testing for the
> roughly equivalent (Character format bitAnd: 16rFFFF) = 0).
> 
> Here is what I measured:
> 
> "Spur 32bit, using the old unoptimized methods (no adoptInstance:)"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 23393
> 
> "Spur 32bit trunk, no special handling"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 1513
> 
> "Spur 32bit, #ascii and #binary with exception handlers for #adoptInstance:"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 2491
> 
> "Spur 32bit, with test for #isRunningSpur rather than exception handler"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 1729
> 
> "V3 on Cog, old version of #ascii and #binary, no special handling"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 47293
> 
> "V3 on Cog, #ascii and #binary with exception handlers for #adoptInstance:"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 5081
> 
> "V3 on Cog, with test for #isRunningSpur rather than exception handler:"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 51091
> 
> "V3 on interpreter, old version of #ascii and #binary, no special handling"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 49139
> 
> "V3 on interpreter, #ascii and #binary with exception handlers for #adoptInstance:"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 55952
> 
> "V3 on interpreter, with test for #isRunningSpur rather than exception handler:"
> Time millisecondsToRun:
>    [ (1 to: 10000000)
>        inject: (FileStream fileNamed: 'delete.me') close    
>        into: [ :fs :idx | fs ascii; binary. fs ] ] ==> 53898
> 
> 
>> 
>> | s |
>> s := ByteString new: 2048.
>> {
>> "Old behavior"
>> [
>>    s := s asByteArray.
>>    s := s asString ].
>> "Simulate failing adoptInstance: with #error. The actual overhead is 
>> higher, because the stack is deeper."
>> [
>>    [ self error ]
>>        on: Error
>>        do: [ s := s asByteArray ].
>>    [ self error ]
>>        on: Error
>>        do: [ s := s asString ] ].
>> "New behavior."
>> [
>>    ByteArray adoptInstance: s.
>>    ByteString adoptInstance: s ].
>> "Proposed workaround."
>> [
>>    [ ByteArray adoptInstance: s ]
>>        on: Error
>>        do: [ s := s asByteArray ].
>>    [ ByteString adoptInstance: s ]
>>        on: Error
>>        do: [ s := s asString ] ].
>> "Alternative workaround."
>> [
>>    Smalltalk isRunningSpur
>>        ifTrue: [ ByteArray adoptInstance: s ]
>>        ifFalse: [ s := s asByteArray ].
>>    Smalltalk isRunningSpur
>>        ifTrue: [ ByteString adoptInstance: s ]
>>        ifFalse: [ s := s asByteString ] ] }
> 
> 
> This needs to be #asString rather than #asByteString.
> 
> 
>>    collect: [ :block |
>>        Smalltalk garbageCollect.
>>        block bench ].
>> #(
>>    '182,000 per second. 5.5 microseconds per run.'
>>    '106,000 per second. 9.43 microseconds per run.'
>>    '10,400,000 per second. 96.6 nanoseconds per run.'
>>    '3,940,000 per second. 254 nanoseconds per run.'
>>    '9,050,000 per second. 110 nanoseconds per run.')
>> 
>> P.P.S.: By using (Character format bitAnd: 16rFFFF) = 0 instead of 
>> Smalltalk isRunningSpur, the overhead of the alternative 
>> solution will disappear on Spur.
> 
> This will work too, although I did not test it. The definition of format
> numbers varies in the different image formats, so #isRunningSpur seems
> good enough, and maybe easier to understand (?).
> 
> Dave
> 
>> 
>>> On Sun, 4 Sep 2016, David T. Lewis wrote:
>>> 
>>> I don't know if this is a good idea, so it goes in the inbox.
>>> 
>>> Dave
>>> 
>>> 
>>>> On Sun, Sep 04, 2016 at 07:09:28PM +0000, commits at source.squeak.org wrote:
>>>> David T. Lewis uploaded a new version of Files to project The Inbox:
>>>> http://source.squeak.org/inbox/Files-dtl.166.mcz
>>>> 
>>>> ==================== Summary ====================
>>>> 
>>>> Name: Files-dtl.166
>>>> Author: dtl
>>>> Time: 4 September 2016, 3:09:29.495898 pm
>>>> UUID: 85945e53-39b3-4300-a08c-d189a4b9960d
>>>> Ancestors: Files-tfel.165
>>>> 
>>>> Let the Files package be  identical for Spur and V3 images by adding 
>>>> error handling in StandardFileStream>>binary and StandardFileStream>>ascii
>>>> 
>>>> =============== Diff against Files-tfel.165 ===============
>