The Inbox: Files-dtl.166.mcz
Locked
 
|
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 =============== Item was changed: ----- Method: StandardFileStream>>ascii (in category 'properties-setting') ----- ascii "Read and/or write in ASCII mode." buffer1 ifNil: [ buffer1 := ByteString new: 1 ] + ifNotNil: [ [ ByteString adoptInstance: buffer1 ] + on: Error "V3 image" + do: [ buffer1 := ByteString new: 1 ] ]. + collection ifNotNil: [ [ ByteString adoptInstance: collection ] + on: Error + do: [ collection := collection asString ] ]. - ifNotNil: [ ByteString adoptInstance: buffer1 ]. - collection ifNotNil: [ ByteString adoptInstance: collection ]. lastWritten ifNotNil: [ lastWritten isInteger ifTrue: [ lastWritten := lastWritten asCharacter ] ]! 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 ] ]! |
Re: The Inbox: Files-dtl.166.mcz
|
|
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, [hidden email] 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 =============== > > Item was changed: > ----- Method: StandardFileStream>>ascii (in category 'properties-setting') ----- > ascii > "Read and/or write in ASCII mode." > buffer1 > ifNil: [ buffer1 := ByteString new: 1 ] > + ifNotNil: [ [ ByteString adoptInstance: buffer1 ] > + on: Error "V3 image" > + do: [ buffer1 := ByteString new: 1 ] ]. > + collection ifNotNil: [ [ ByteString adoptInstance: collection ] > + on: Error > + do: [ collection := collection asString ] ]. > - ifNotNil: [ ByteString adoptInstance: buffer1 ]. > - collection ifNotNil: [ ByteString adoptInstance: collection ]. > lastWritten ifNotNil: > [ lastWritten isInteger ifTrue: [ lastWritten := lastWritten asCharacter ] ]! > > 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 ] ]! > |
Re: The Inbox: Files-dtl.166.mcz
|
|
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 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. | 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 ] ] } 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. 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, [hidden email] 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 =============== >> >> Item was changed: >> ----- Method: StandardFileStream>>ascii (in category 'properties-setting') ----- >> ascii >> "Read and/or write in ASCII mode." >> buffer1 >> ifNil: [ buffer1 := ByteString new: 1 ] >> + ifNotNil: [ [ ByteString adoptInstance: buffer1 ] >> + on: Error "V3 image" >> + do: [ buffer1 := ByteString new: 1 ] ]. >> + collection ifNotNil: [ [ ByteString adoptInstance: collection ] >> + on: Error >> + do: [ collection := collection asString ] ]. >> - ifNotNil: [ ByteString adoptInstance: buffer1 ]. >> - collection ifNotNil: [ ByteString adoptInstance: collection ]. >> lastWritten ifNotNil: >> [ lastWritten isInteger ifTrue: [ lastWritten := lastWritten asCharacter ] ]! >> >> 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 ] ]! >> > > |
Re: The Inbox: Files-dtl.166.mcz
|
|
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, [hidden email] 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 =============== > >> |
Re: The Inbox: Files-dtl.166.mcz
|
|
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, [hidden email] 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 =============== >>>> > > |
Re: The Inbox: Files-dtl.166.mcz
|
|
In reply to this post by David T. Lewis
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 <[hidden email]> 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, [hidden email] 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 =============== > |
Re: The Inbox: Files-dtl.166.mcz
|
|
On Mon, Sep 05, 2016 at 02:33:43AM +0200, Eliot Miranda wrote:
> 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. > Hi Eliot, Right. You have documented this clearly in VMMaker. It is a reasonable and expected limitation. > 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. My initial attempt to do this is http://www.squeaksource.com/TrunkUpdateStreamV3. For example, to see differences in the Collections package, use Monticello from trunk Squeak to look at differences in the Collections.V3 package in the TrunkUpdateStreamV3 repository. The other differences are in Compiler.V3, Kernel.V3, and System.V3 (along with the trivial difference in Files.V3 that was the subject of this thread). My question concerning the Files package arises directly from this. Aside from a single optimization in Files, all other significant differences have been contained within the Compiler, Collections, System, and Kernel packages. > Instead one could put effort into a Spur interpreter VM, I do not want to volunteer to start a new VM project until I finish the last one that I committed to do. I have said this before. And it's really just not someething that I can take on right now. > , or tools for automatic patching of Monticello packages with Spur<->V3 differences. I do not yet have any real insight as to how to automate the patching of Monticello packages with Spur<->V3 differences. So far I have done it manually with some success, and I have been able to maintain a functioning update.V3 config map for the V3 trunk update stream (the update.V3 maps in http://www.squeaksource.com/TrunkUpdateStreamV3). This has been updated through the 5.1 release cycle, and I may keep it going for a while longer. But the automated update stream test (http://build.squeak.org/job/FollowTrunkOnOldV3Image) fails due to various issues in the trunk stream itself (discussed elsewhere), and my experience is that the changes in System/Kernel/Collections/Compiler require close inspection by a human agent. I don't know how to automate this. I can't help but think this is a topic that might usefully be addressed with Environments. That is pure hand waving on my part, but it seems like it might be a natural fit, and potentially better in the long term than any Monticello patching strategies that we may be able to invent. I am also intrigued by Juan Vuletich's idea of maintaining a single image code base for both V3 and Spur, so if Juan can do it it must be possible :-) http://cuis-smalltalk.org/pipermail/cuis-dev_cuis-smalltalk.org/2016-August/000854.html > 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). > I agree completely. > 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) > I have heard this before. It would be "too much work" to keep squeaksource.com alive, so instead we direct all available resources to migrate everything to smalltalkhub. It would be "too hard" to keep MVC working, so we should just abandon it while we work on something else instead. Sorry, but I just don't buy it. For various reasons, I cannot put much effort into new projects right now. But I do want to try to keep existing efforts moving forward, and I hope this will not be seen as unconstructive. Dave |
|
|
On Mon, Sep 5, 2016 at 11:38 AM, David T. Lewis <[hidden email]> wrote:
> On Mon, Sep 05, 2016 at 02:33:43AM +0200, Eliot Miranda wrote: >> 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. >> > > Hi Eliot, > > Right. You have documented this clearly in VMMaker. It is a reasonable and > expected limitation. > >> 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. > > My initial attempt to do this is http://www.squeaksource.com/TrunkUpdateStreamV3. > For example, to see differences in the Collections package, use Monticello from > trunk Squeak to look at differences in the Collections.V3 package in the > TrunkUpdateStreamV3 repository. The other differences are in Compiler.V3, > Kernel.V3, and System.V3 (along with the trivial difference in Files.V3 that > was the subject of this thread). > > My question concerning the Files package arises directly from this. Aside > from a single optimization in Files, all other significant differences > have been contained within the Compiler, Collections, System, and Kernel > packages. > >> Instead one could put effort into a Spur interpreter VM, > > I do not want to volunteer to start a new VM project until I finish the > last one that I committed to do. I have said this before. And it's really > just not someething that I can take on right now. Dave, Just to clarify... If there happen to be a Context Interpreter Spur VM, you would not be concerned about V3 compatibility? cheers -ben |
Re: The Inbox: Files-dtl.166.mcz
|
|
On Mon, Sep 05, 2016 at 01:08:38PM +0800, Ben Coman wrote:
> On Mon, Sep 5, 2016 at 11:38 AM, David T. Lewis <[hidden email]> wrote: > > On Mon, Sep 05, 2016 at 02:33:43AM +0200, Eliot Miranda wrote: > >> 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. > >> > > > > Hi Eliot, > > > > Right. You have documented this clearly in VMMaker. It is a reasonable and > > expected limitation. > > > >> 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. > > > > My initial attempt to do this is http://www.squeaksource.com/TrunkUpdateStreamV3. > > For example, to see differences in the Collections package, use Monticello from > > trunk Squeak to look at differences in the Collections.V3 package in the > > TrunkUpdateStreamV3 repository. The other differences are in Compiler.V3, > > Kernel.V3, and System.V3 (along with the trivial difference in Files.V3 that > > was the subject of this thread). > > > > My question concerning the Files package arises directly from this. Aside > > from a single optimization in Files, all other significant differences > > have been contained within the Compiler, Collections, System, and Kernel > > packages. > > > >> Instead one could put effort into a Spur interpreter VM, > > > > I do not want to volunteer to start a new VM project until I finish the > > last one that I committed to do. I have said this before. And it's really > > just not someething that I can take on right now. > > Dave, Just to clarify... If there happen to be a Context Interpreter > Spur VM, you would not be concerned about V3 compatibility? > Hi Ben, Ideally we would like to be able to have the context and stack interpreters built from the same code base, and in that scenario the context interpreter would be of little interest for a Spur object memory. The stack interpreter already exists for both V3 and Spur, and it is an improvement on the traditional context interpreter. That said, there are things in the VMMaker code base for the interpreter VM that could usefully be moved to the VMMaker.oscog code base, and Eliot is quite rightly trying to encourage someone to step forward and make that happen. If anyone is interested, I'll be happy to help where I can (but it's not a project that I want to try to undertake myself right now). Off the top of my head, here are some things that might be of interest from the VMMaker code base: - Refactor interpreter and object memory hierarchies, methods specific to context versus stack interpreters in their own hierarchies, no method overrides, use #subclassResponsibility in common superclasses - For methods specific to the context interpreter, update them from the more recent code in VMMaker - 32 versus 64 bit object word size as a compile time option (not as part of source code generation), single generated C code base - Ensure that context and stack interpreters (not necessarily Cog) compile and run on 64-bit platforms (no 32-bit libraries) - Add support for loading older 6504 images (should work with stack interpreter) - Memory access C macros reimplemented in slang, allow similator to execute the lowest level functions in Smalltalk - Browse generated C code for methods in Squeak browsers Dave |
Re: The Inbox: Files-dtl.166.mcz
|
|
Hi David,
On Mon, Sep 5, 2016 at 7:14 AM, David T. Lewis <[hidden email]> wrote:
Thanks for this list! I agree with all of it except where I've commented. If anyone is interested, I'll be happy to help where I can (but it's not This is infeasible. The way Spur 64- vs 32- bit is architected is different from V3. There are distinct subclasses of SpurMemoryManager (Spur32BitMemoryManager, Spur32BitCoMemoryManager, Spur64BitMemoryManager, Spur64BitCoMemoryManager) which works well in being able to optimize the code for each variant, but makes it very difficult to generate #if SPUR64 style code. Further, the generation of the Cogit is one file per processor. The separate generation works fine. We have compile time options for fairly small changes, such as IMMUTABILITY and Pharo vs Squeak (which affects the FilePlugin). But for something as pervasive as 64- vs 32- bits it's not practical and IMO of little benefit. - Ensure that context and stack interpreters (not necessarily Cog) compile and just to be clear, start the work from the oscog fork, not the VMMaker trunk, since so many changes have been made to Slang to get the Cog and Spur systems going. Dave _,,,^..^,,,_ best, Eliot |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |