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

David T. Lewis lewis at mail.msen.com
Mon Sep 5 00:12:01 UTC 2016


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