Smalltalk › Pharo › Pharo Smalltalk Developers

SmallInteger #printString versus #printOn:

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

5 messages Options

Ben Coman

SmallInteger #printString versus #printOn:

I was going to slip this example into the welcome help,

1 printString [debugIt]

and I'm confronted by SmallInteger>>printString

printString

"Highly optimized version for base 10

and that we know it is a SmallInteger."

| integer next result len |

self = 0 ifTrue: [^'0'].

self < 0 ifTrue: [^'-', self negated printString].

len := self decimalDigitLength.

result := String new: len.

integer := self.

len to: 1 by: -1 do: [:i |

next := integer // 10.

result byteAt: i put: 48 + (integer - (next * 10)).

integer := next].

^result

and I gather from other discussions this should(?) belong in SmallInteger>>printOn:

Or is there a large benefit to avoiding the #printStringLimitedTo: machinery ?

cheers -ben

Nicolas Cellier

Re: SmallInteger #printString versus #printOn:

2017-04-08 6:38 GMT+02:00 Ben Coman <[hidden email]>:

I was going to slip this example into the welcome help,

1 printString [debugIt]

and I'm confronted by SmallInteger>>printString

printString
"Highly optimized version for base 10
and that we know it is a SmallInteger."

| integer next result len |
self = 0 ifTrue: [^'0'].
self < 0 ifTrue: [^'-', self negated printString].
len := self decimalDigitLength.
result := String new: len.
integer := self.
len to: 1 by: -1 do: [:i |
next := integer // 10.
result byteAt: i put: 48 + (integer - (next * 10)).
integer := next].
^result

and I gather from other discussions this should(?) belong in SmallInteger>>printOn:
Or is there a large benefit to avoiding the #printStringLimitedTo: machinery ?

cheers -ben

It should be printOn: generally, except that printOn: requires creating a Stream, and requires passing thru Stream indirection for writing into the target String.

As the comment tells, the sole reason for this implementation is "Highly optimized version". So consider that it's an exception to the rules. Maybe this kind of decision could be periodically re-examined in the light of performance measurements (I'm thinking of Sista)?

Nicolas Cellier

Re: SmallInteger #printString versus #printOn:

Ah, but now that I've put a
(selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self halt: 'debug me'].
I can't reproduce the problem related to removal of Context methods!!!

Instead, I've got the 'Merging Kernel-eem.1078' window with all the MethodContext->Context renaming in conflict
I already reported that in another thread where I told that the upgrade went "smoothly" for both my 32 & 64 bits images.
I don't know if it's related to package cache, or .mcd, or ???

It might also be that package ancestry is not correct du to overwriting of some packages, otherwise we should not get a merge window, but well...

Ah, but now I think I understand: it's just because I have pending changes in Kernel due to the addition of halt: above (same for my images which usually have some unpublished experiments).
We then trigger a merge rather than a load. And the merge sees conflict probably because of broken ancestry (the GUID are not corresponding), but once resolved manually, it processes smoothly without removing Context methods...

I can't instrument code that easily... This will have to wait for another time slot :(

2017-04-08 13:41 GMT+02:00 Nicolas Cellier <[hidden email]>:

2017-04-08 6:38 GMT+02:00 Ben Coman <[hidden email]>:
I was going to slip this example into the welcome help,

1 printString [debugIt]

and I'm confronted by SmallInteger>>printString

printString
"Highly optimized version for base 10
and that we know it is a SmallInteger."

| integer next result len |
self = 0 ifTrue: [^'0'].
self < 0 ifTrue: [^'-', self negated printString].
len := self decimalDigitLength.
result := String new: len.
integer := self.
len to: 1 by: -1 do: [:i |
next := integer // 10.
result byteAt: i put: 48 + (integer - (next * 10)).
integer := next].
^result

and I gather from other discussions this should(?) belong in SmallInteger>>printOn:
Or is there a large benefit to avoiding the #printStringLimitedTo: machinery ?

cheers -ben

It should be printOn: generally, except that printOn: requires creating a Stream, and requires passing thru Stream indirection for writing into the target String.

As the comment tells, the sole reason for this implementation is "Highly optimized version". So consider that it's an exception to the rules. Maybe this kind of decision could be periodically re-examined in the light of performance measurements (I'm thinking of Sista)?

Nicolas Cellier

Re: SmallInteger #printString versus #printOn:

Hmm forget about this, wrong thread...

2017-04-08 13:58 GMT+02:00 Nicolas Cellier <[hidden email]>:

Ah, but now that I've put a
(selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self halt: 'debug me'].
I can't reproduce the problem related to removal of Context methods!!!

Instead, I've got the 'Merging Kernel-eem.1078' window with all the MethodContext->Context renaming in conflict
I already reported that in another thread where I told that the upgrade went "smoothly" for both my 32 & 64 bits images.
I don't know if it's related to package cache, or .mcd, or ???

It might also be that package ancestry is not correct du to overwriting of some packages, otherwise we should not get a merge window, but well...

Ah, but now I think I understand: it's just because I have pending changes in Kernel due to the addition of halt: above (same for my images which usually have some unpublished experiments).
We then trigger a merge rather than a load. And the merge sees conflict probably because of broken ancestry (the GUID are not corresponding), but once resolved manually, it processes smoothly without removing Context methods...

I can't instrument code that easily... This will have to wait for another time slot :(

2017-04-08 13:41 GMT+02:00 Nicolas Cellier <[hidden email]>:

2017-04-08 6:38 GMT+02:00 Ben Coman <[hidden email]>:
I was going to slip this example into the welcome help,

1 printString [debugIt]

and I'm confronted by SmallInteger>>printString

printString
"Highly optimized version for base 10
and that we know it is a SmallInteger."

| integer next result len |
self = 0 ifTrue: [^'0'].
self < 0 ifTrue: [^'-', self negated printString].
len := self decimalDigitLength.
result := String new: len.
integer := self.
len to: 1 by: -1 do: [:i |
next := integer // 10.
result byteAt: i put: 48 + (integer - (next * 10)).
integer := next].
^result

and I gather from other discussions this should(?) belong in SmallInteger>>printOn:
Or is there a large benefit to avoiding the #printStringLimitedTo: machinery ?

cheers -ben

It should be printOn: generally, except that printOn: requires creating a Stream, and requires passing thru Stream indirection for writing into the target String.

As the comment tells, the sole reason for this implementation is "Highly optimized version". So consider that it's an exception to the rules. Maybe this kind of decision could be periodically re-examined in the light of performance measurements (I'm thinking of Sista)?

Stephane Ducasse-3

Re: SmallInteger #printString versus #printOn:

In reply to this post by Nicolas Cellier

but I guess that the printOn: method should still be working because we can invoke it instead of creating a new stream.

I mean as a developer I do not know that printString is not creating a stream so normally I should use printOn: especially from other printOn: method to avoid the creation of spurious streams. Now this printstring method is just good to printString and asString (if it invokes it). May be smallInteger>>printOn: call it too if it does not I have the impression that it should.

On Sat, Apr 8, 2017 at 1:41 PM, Nicolas Cellier <[hidden email]> wrote:

2017-04-08 6:38 GMT+02:00 Ben Coman <[hidden email]>:
I was going to slip this example into the welcome help,

1 printString [debugIt]

and I'm confronted by SmallInteger>>printString

printString
"Highly optimized version for base 10
and that we know it is a SmallInteger."

| integer next result len |
self = 0 ifTrue: [^'0'].
self < 0 ifTrue: [^'-', self negated printString].
len := self decimalDigitLength.
result := String new: len.
integer := self.
len to: 1 by: -1 do: [:i |
next := integer // 10.
result byteAt: i put: 48 + (integer - (next * 10)).
integer := next].
^result

and I gather from other discussions this should(?) belong in SmallInteger>>printOn:
Or is there a large benefit to avoiding the #printStringLimitedTo: machinery ?

cheers -ben

It should be printOn: generally, except that printOn: requires creating a Stream, and requires passing thru Stream indirection for writing into the target String.

As the comment tells, the sole reason for this implementation is "Highly optimized version". So consider that it's an exception to the rules. Maybe this kind of decision could be periodically re-examined in the light of performance measurements (I'm thinking of Sista)?