[squeak-dev] The Inbox: Files-dtl.166.mcz

Levente Uzonyi leves at caesar.elte.hu
Mon Sep 5 00:29:44 UTC 2016


Hi Dave,

Now I remember what the problem was with #adoptInstance: on V3. (Based on 
your benchmark results on V3 Cog, it has already been fixed in your 
image.)
The thing is that ByteString has always been compact, but ByteArray has 
never been, so their format is different in V3. In Spur there are no 
compact classes, so this issue doesn't exist. The solution was to evaluate 
[ ByteArray becomeCompact ] in the V3 image.
But you reported that the interpreter VM's primitive for #adoptInstance: 
is broken[1].
So, I think the best would be if that primitive were fixed in the 
interpreter VM.

Levente

[1] http://forum.world.st/Files-ul-154-seems-dangerous-is-it-needed-td4898213.html

On Sun, 4 Sep 2016, David T. Lewis 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 ===============
>>>>
>
>


More information about the Squeak-dev mailing list