Issue 3429 in pharo: Better Number performance

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 3429 in pharo: Better Number performance

pharo
Status: FixedWaitingToBePharoed
Owner: stephane.ducasse
Labels: Milestone-1.3

New issue 3429 by stephane.ducasse: Better Number performance
http://code.google.com/p/pharo/issues/detail?id=3429

Name: Kernel-ul.522
Author: ul
Time: 8 December 2010, 4:08:19.384 am
UUID: 6927b03c-21df-1746-9a0d-115b717a5e40
Ancestors: Kernel-ul.521

- implemented Fraction >> #negative and ScaledDecimal >> #negative for  
better performance. These methods don't allocate new objects.
- reimplemented SmallInteger printing methods (printOn:base:*) without  
String allocation. They are as fast as the previous versions were for base  
10.
- reimplemented ScaledDecimal printing. It avoids object allocations and  
has better performance than the previous version. ScaledDecimals can also  
be printed without the scale part by using #printFractionAsDecimalOn:.

=============== Diff against Kernel-ul.521 ===============

Item was added:
+ ----- Method: Fraction>>negative (in category 'testing') -----
+ negative
+
+       ^numerator negative!

Item was added:
+ ----- Method: ScaledDecimal>>negative (in category 'testing') -----
+ negative
+
+       ^fraction negative!

Item was added:
+ ----- Method: ScaledDecimal>>printFractionAsDecimalOn: (in  
category 'printing') -----
+ printFractionAsDecimalOn: stream
+
+       | aFraction integerPart fractionPart |
+       fraction negative
+               ifFalse: [ aFraction := fraction ]
+               ifTrue: [
+                       aFraction := fraction negated.
+                       stream nextPut: $- ].
+       integerPart := aFraction truncated.
+       integerPart printOn: stream.
+       scale = 0 ifTrue: [ ^self ].
+       stream nextPut: $..
+       fractionPart := ((aFraction - integerPart) * (10 raisedToInteger:  
scale)) truncated.
+       fractionPart
+               printOn: stream
+               base: 10
+               length: scale
+               padded: true!

Item was changed:
  ----- Method: ScaledDecimal>>printOn: (in category 'printing') -----
+ printOn: stream
+
+       self
+               printFractionAsDecimalOn: stream;
+               printScaleOn: stream!
- printOn: aStream
-       "Reimplementation - Object 'printing' method."
-       | aFraction tmpFractionPart |
-       self < 0 ifTrue: [aStream nextPut: $-].
-       aFraction := fraction abs.
-       aStream nextPutAll: aFraction truncated printString.
-       scale = 0 ifTrue: [^ aStream nextPutAll: 's0'].
-       aStream nextPut: $..
-       tmpFractionPart := aFraction fractionPart.
-       1 to: scale
-               do:
-                       [:dummy |
-                       tmpFractionPart := tmpFractionPart * 10.
-                       aStream nextPut: (Character digitValue:  
tmpFractionPart truncated).
-                       tmpFractionPart := tmpFractionPart fractionPart].
-       aStream nextPut: $s.
-       scale printOn: aStream!

Item was added:
+ ----- Method: ScaledDecimal>>printScaleOn: (in category 'printing') -----
+ printScaleOn: stream
+
+       stream nextPut: $s.
+       scale printOn: stream!

Item was changed:
  ----- Method: SmallInteger>>printOn:base: (in category 'printing') -----
+ printOn: stream base: base
- printOn: aStream base: b
        "Append a representation of this number in base b on aStream."

+       self printOn: stream base: base length: 0 padded: false!
-       self < 0
-               ifTrue: [aStream nextPut: $-.
-                       aStream nextPutAll: (self negated printStringBase:  
b).
-                       ^self].
-
-       "allocating a String seems faster than streaming for SmallInteger"
-       aStream nextPutAll: (self printStringBase: b)!

Item was added:
+ ----- Method: SmallInteger>>printOn:base:length:padded: (in  
category 'printing') -----
+ printOn: stream base: base length: minimumLength padded: padWithZeroes
+
+       | n numberOfDigits totalLength divisor |
+       self < 0
+               ifTrue: [
+                       n := self negated.
+                       totalLength := 1 ]
+               ifFalse: [
+                       n := self.
+                       totalLength := 0 ].
+       numberOfDigits := n numberOfDigitsInBase: base.
+       totalLength := totalLength + numberOfDigits.
+       padWithZeroes ifFalse: [
+               [ totalLength < minimumLength ] whileTrue: [
+                       stream space.
+                       totalLength := totalLength + 1 ] ].
+       n = self ifFalse: [ stream nextPut: $- ].
+       padWithZeroes ifTrue: [
+               [ totalLength < minimumLength ] whileTrue: [
+                       stream nextPut: $0.
+                       totalLength := totalLength + 1 ] ].
+       divisor := (base raisedToInteger: numberOfDigits - 1).
+       [ divisor > 0 ] whileTrue: [
+               | digit |
+               digit := n // divisor.
+               stream nextPut: ('0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ' at:  
digit + 1).
+               n := n - (digit * divisor).
+               divisor := divisor // base ]!

Item was changed:
  ----- Method: SmallInteger>>printOn:base:nDigits: (in category 'printing')  
-----
  printOn: aStream base: b nDigits: n
        "Append a representation of this number in base b on aStream using  
nDigits.
        self must be positive."

+       self printOn: aStream base: b length: n padded: true!
-       "allocating a String seems faster than streaming for SmallInteger"
-       aStream nextPutAll: (self printStringBase: b nDigits: n)!



Reply | Threaded
Open this post in threaded view
|

Re: Issue 3429 in pharo: Better Number performance

pharo
Updates:
        Status: FixProposed

Comment #1 on issue 3429 by [hidden email]: Better Number performance
http://code.google.com/p/pharo/issues/detail?id=3429

Name: SLICE-Issue-3429-BetterNumberPerformance-nice.1
Dependencies: Kernel-nice.852

Integrate Levente's fast up (Fraction negative and SmallInteger printOn:)
For ScaledDecimal, this is different, because they are printed rounded in  
Pharo, truncated in Squeak.

BTW, Pharo version was wrong (shame on me) because rounded fractionPart can  
overflow and this was not taken into account (try 1.999s2 printString ->  
Error)
So for the same SLICE, you got a patch.

TODO: someone might want to add a non regression test for 1.999s2  
printString = '2.00s2'
And also test -1.999s2 printString = '-2.00s2'


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3429 in pharo: Better Number performance

pharo
Updates:
        Labels: Type-Squeak

Comment #2 on issue 3429 by [hidden email]: Better Number performance
http://code.google.com/p/pharo/issues/detail?id=3429

(No comment was entered for this change.)


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3429 in pharo: Better Number performance

pharo
Updates:
        Status: Closed

Comment #3 on issue 3429 by [hidden email]: Better Number performance
http://code.google.com/p/pharo/issues/detail?id=3429

Thanks nicolas.
In this moment we have some boost in changes so this is good to have some  
help :).
I like the feel.

I added ScaledDecimalTest>>testRounded

        self assert: 1.999s2 printString = '2.00s2'.
        self assert: -1.999s2 printString = '-2.00s2'


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3429 in pharo: Better Number performance

pharo

Comment #4 on issue 3429 by [hidden email]: Better Number performance
http://code.google.com/p/pharo/issues/detail?id=3429

in 13118


Reply | Threaded
Open this post in threaded view
|

Re: Issue 3429 in pharo: Better Number performance

Nicolas Cellier
Sorry, it seems I let another bug, like -0.01s2 printString loosing its sign.
There might be some tests missing in KernelTests-Numbers

I will open a new issue and give a new patch from trunk.

Nicolas

2011/3/29  <[hidden email]>:
>
> Comment #4 on issue 3429 by [hidden email]: Better Number performance
> http://code.google.com/p/pharo/issues/detail?id=3429
>
> in 13118
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Issue 3429 in pharo: Better Number performance

Stéphane Ducasse
Thanks.
No stress :). even if the minus can make all the difference :)

Stef

> Sorry, it seems I let another bug, like -0.01s2 printString loosing its sign.
> There might be some tests missing in KernelTests-Numbers
>
> I will open a new issue and give a new patch from trunk.
>
> Nicolas
>
> 2011/3/29  <[hidden email]>:
>>
>> Comment #4 on issue 3429 by [hidden email]: Better Number performance
>> http://code.google.com/p/pharo/issues/detail?id=3429
>>
>> in 13118
>>
>>
>>
>