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

David T. Lewis lewis at mail.msen.com
Sat Oct 26 20:55:51 UTC 2019


Levente,

Thanks for looking at this :-)

On Fri, Oct 25, 2019 at 03:22:32PM +0200, Levente Uzonyi wrote:
> Hi Dave,
> 
> While error handling seems to be a straigforward fix, the reason why 
> #adoptInstance: is used there is to:
> 1. be as quick as possible
> 2. avoid garbage creation (to achieve 1)
> An error handler works against those goals.

OK. But a question, off topic and just for my own understanding - I was
expecting that the error handler would mainly add overhead in the case
of handling a failure (the V3 image case), and would add little overhead
in the normal case (Spur) for which the adoptInstance: is successful.
I was not worried about performance in the V3 case (because #binary
is not frequently called so AFAICT it does not matter), so this seemed
like a reasonable solution.

>From a performance perspective, is it better to avoid using on:do: even
if one expects the block to succeed in the common case?

> Using pure #asByteArray, which is even more straightforward, would be a 
> better choice, as it would be faster and would create no or less garbage 
> than an error handler. If you have a look at the method's history, you'll 
> see that was used before #adoptInstance:.

You're right, the 'eem 5/27/2014' version of the method would be a
better solution overall. But if using adoptInstance is important,
then we should just leave things as they are.

> 
> There is also the complementary method, #ascii, which does the same 
> conversion but to ByteString instead of to ByteArray. I suppose that 
> method also doesn't work on V3.

The #ascii implementation is not a concern. ByteString can adopt an
instance of ByteArray on V3.

> 
> IIRC, the V3 VMs support primitive 115 (#primitiveChangeClassTo:). If they 
> do, I'd rather change #adoptInstance:'s fallback code to try 
> #primitiveChangeClassTo: (preferably only on V3 to avoid any side 
> effects). That way other users of #adoptInstance: could work on V3.

Both primitives have the same constraint on V3. And, in fact, the
earlier fallback code (hsj 2/16/2011) does what you suggest:

Behavior>>adoptInstance: anInstance
	"Change the class of anInstance to me.
	Primitive (found in Cog and new VMs)  follows the same rules as primitiveChangeClassTo:, but returns the class rather than the modified instance"

	<primitive: 160 error: ec>
	anInstance primitiveChangeClassTo: self basicNew.
	^self

But this does not help because primitive 115 fails for the same reason
as primitive 160.

Again, thank you for looking at this. This is not an important problem
to fix, just a bit of an annoyance for me in keeping track of the V3/Spur
differences.

Dave

> 
> Levente
> 
> On Fri, 25 Oct 2019, 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.184.mcz
> >
> >==================== Summary ====================
> >
> >Name: Files-dtl.184
> >Author: dtl
> >Time: 24 October 2019, 10:15:34.856587 pm
> >UUID: cfa02841-644b-49cb-b3f2-a0eb1b36707b
> >Ancestors: Files-pre.183
> >
> >Provide error handling for #adoptInstance in StandardFileStream>>binary to 
> >provide pre-Spur behavior when running on a V3 image. Prior to Spur, class 
> >ByteArray cannot adopt an instance of ByteString, so use #asByteArray 
> >instead.
> >
> >Resolves the only remaining difference for package Files when running on 
> >either Spur or V3 images. The packages with significant Spur/V3 
> >differences (excluding unit tests) are now Collections, Compiler, Kernel, 
> >and System. See www.squeaksource.com/TrunkUpdateStreamV3 for verification.
> >
> >=============== Diff against Files-pre.183 ===============
> >
> >Item was changed:
> > ----- Method: StandardFileStream>>binary (in category 
> > 'properties-setting') -----
> > binary
> > 	"Read and/or write in binary mode."
> > 	buffer1
> > 		ifNil: [ buffer1 := ByteArray new: 1 ]
> >+ 		ifNotNil: [ [ ByteArray adoptInstance: buffer1 ]
> >+ 						on: Error "V3 image"
> >+ 						do: [ buffer1 := ByteArray 
> >new: 1 ] ].
> >+ 	collection ifNotNil: [ [ ByteArray adoptInstance: collection ]
> >+ 							on: Error
> >+ 							do: [ collection := 
> >collection asByteArray ] ].
> >- 		ifNotNil: [ ByteArray adoptInstance: buffer1 ].
> >- 	collection ifNotNil: [ ByteArray adoptInstance: collection ].
> > 	lastWritten ifNotNil:
> > 		[ lastWritten isCharacter ifTrue: [ lastWritten := 
> > 		lastWritten asInteger ] ]!
> 


More information about the Squeak-dev mailing list