Re: [Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-31 Thread Henrik Johansen

> On 31 May 2016, at 11:44 , Holger Freyther  wrote:
> 
> not with Pharo5:
> 
> ((LargePositiveInteger new: 3) digitAdd: SmallInteger maxVal - 10) class
> => SmallInteger

That's a very recent change to the LargeInteger primitives, wasn't in my old 
spur vm (a month old perhaps).
So,in addition to #digitAdd: being 3 times slower than #+ (...jittability? 
Shouldn't be *that* much more work to add a byte to another byte compared to 
whatever #+ does)
nor is it hitting the fast path for replaceFrom:.. (2 variableX instances) 
either, meaning it'll see an even bigger improvement from reverting compared to 
Pharo4 (which got 50% faster...)

> '
> 
> cool! Change looks good but please remove the obsolete >>#digitAdd: comment.
> 
> thanks a lot!!

Done, published as .47
Tested counter increments and reset at image startup, seemed to work at least.

Cheers,
Henry


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: [Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-31 Thread Holger Freyther

> On 30 May 2016, at 19:03, Henrik Johansen  
> wrote:
> 

Hi!


> It's starting to come back to me; IIRC, + will normalize results to 
> SmallIntegers, digitAdd: will not.

not with Pharo5:

((LargePositiveInteger new: 3) digitAdd: SmallInteger maxVal - 10) class
=> SmallInteger



> I thought it would be nice to use a single
> replaceFrom:to:with:startingAt:
> call to insert the entire counter; however, I didn't bench that particular 
> part.

yes, it is nice


> So while the rewrite overall gained a small amount of speed, it turns out 
> digitAdd: is quite slow (even though it's a primitive), so reverting to using 
> Smallinteger arithmetic for the counter, and inserting the counter a digit at 
> a time is most likely worth it:
> 
> "Pharo4, LargeInteger counter"
> [OID new] bench '1,194,004 per second'
> 
> "Pharo4, reverted to SmallInteger counter"
> [OID new] bench '1,681,203 per second'

cool! Change looks good but please remove the obsolete >>#digitAdd: comment.

thanks a lot!!

holger


Re: [Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-30 Thread Henrik Johansen

> On 30 May 2016, at 5:49 , Henrik Johansen  
> wrote:
> 
> A few more comments below; I'm not seeing the things you describe when 
> testing in my image...
> 
>>> 
>>> | id |
>>> id := LargePositiveInteger.
>>> 1 to: (16777215 + 50) do: [:each |
>>> id := id digitAdd: 1].
>>> id.
>>> 
>>> Given the comment it should overflow and the value be 50? This is not what 
>>> the result is. So shall the truncation be added again or at least the 
>>> comment be updated? The code will go from LargePositiveInteger to 
>>> SmallInteger when overflowing from three to four bytes
> 
> On which VM does it overflow to SmallInteger? I've never seen this, on the 
> one I tested the expression in last mail, the "overflowing" digitAdd: 
> returned a 4-byte LargePositiveInteger.

It's starting to come back to me; IIRC, + will normalize results to 
SmallIntegers, digitAdd: will not.

I thought it would be nice to use a single
replaceFrom:to:with:startingAt:
call to insert the entire counter; however, I didn't bench that particular part.

So while the rewrite overall gained a small amount of speed, it turns out 
digitAdd: is quite slow (even though it's a primitive), so reverting to using 
Smallinteger arithmetic for the counter, and inserting the counter a digit at a 
time is most likely worth it:

"Pharo4, LargeInteger counter"
[OID new] bench '1,194,004 per second'

"Pharo4, reverted to SmallInteger counter"
[OID new] bench '1,681,203 per second'

The relevant changes should be present in attachment.

Cheers,
Henry



OIDrevertCounter.cs
Description: Binary data






signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: [Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-30 Thread Henrik Johansen

> On 30 May 2016, at 5:49 , Henrik Johansen  
> wrote:
>>> 
>>> PS: The other part is that >>#newCounter doesn't seem to be ever executed. 
>>> On first load >>#initialize will call >>#reset and on >>#shutDown: calls 
>>> reset. So the code to "randomize" the initial counter doesn't seem to work.
> 
> What?
> OID class >> #initialize adds the class to the shutdown list, so it *should* 
> call shutDown: whenever the image is saved&quit.
> newCounter should then be called on first request after startup, since 
> machineIdentifier is set to nil by reset.


/doh, I see it now.
reset should do

counter := nil, not
counter := LargePositiveInteger new: 3 .

PEBKAC when rewriting the old

counter := 0

reset code, I'm afraid :/


Cheers,
Henry



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: [Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-30 Thread Henrik Johansen
A few more comments below; I'm not seeing the things you describe when testing 
in my image...

>> 
>> | id |
>> id := LargePositiveInteger.
>> 1 to: (16777215 + 50) do: [:each |
>>  id := id digitAdd: 1].
>> id.
>> 
>> Given the comment it should overflow and the value be 50? This is not what 
>> the result is. So shall the truncation be added again or at least the 
>> comment be updated? The code will go from LargePositiveInteger to 
>> SmallInteger when overflowing from three to four bytes

On which VM does it overflow to SmallInteger? I've never seen this, on the one 
I tested the expression in last mail, the "overflowing" digitAdd: returned a 
4-byte LargePositiveInteger.


>> but luckily
>> 
 #value
>>  ...
>>  replaceFrom: 1 to: 3with: self class counterNext startingAt: 1
>> 
>> will even work when counterNext returns a SmallInteger. But given the old 
>> code and the comment in the new code this is does not seem to function as 
>> intended?
>> 

Are you sure?
On my VM,
(ByteArray new: 3) replaceFrom: 1 to: 3 with: 16r1FF startingAt: 1
gives an "Instances of SmallInteger are not indexable" error...

>> 
>> 
>> PS: The other part is that >>#newCounter doesn't seem to be ever executed. 
>> On first load >>#initialize will call >>#reset and on >>#shutDown: calls 
>> reset. So the code to "randomize" the initial counter doesn't seem to work.

What?
OID class >> #initialize adds the class to the shutdown list, so it *should* 
call shutDown: whenever the image is saved&quit.
newCounter should then be called on first request after startup, since 
machineIdentifier is set to nil by reset.

Cheers,
Henry





signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: [Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-30 Thread Henrik Johansen
Hi Holger, sorry, I'm not good at responding to unsorted email in a timely 
manner :/

The comment is inaccurate, the wraparound relates to how the number is used in 
OID >> #initialize, not the number itself.

lp := LargePositiveInteger new: 3.
lp at: 1 put: 255; at: 2 put: 255; at: 3 put: 255.
lp := lp digitAdd: 1.
(ByteArray new: 3) replaceFrom: 1 to: 3 with: lp startingAt: 1; yourself

So the number will keep on growing, but for the purposes of our use, it wraps 
around.

Cheers,
Henry


> On 30 May 2016, at 9:57 , Holger Freyther  wrote:
> 
> Hi,
> 
> I tried to reach the author for several weeks but he doesn't seem to respond 
> so I am trying to reach a wider audience to either confirm my suspicion or to 
> be corrected. In
> http://smalltalkhub.com/#!/~MongoTalkTeam/mongotalk/diff/Mongo-BSON-HenrikSperreJohansen.43
> 
> the following change is done:
> 
> + "digitAdd: wraps around to 0 when result would overflow"
> + ^ counter := counter
> + ifNil:[self newCounter]
> + ifNotNil:[counter digitAdd: 1]!
> - ^ counter := (counter + 1) \\ 16rFF!
> 
> The old code has overflow checking, the new code makes a statement I don't 
> think is true. counter is LargePositiveInteger new: 3 to use three bytes. So 
> given the above code and the experiment (yes I could just add a bigger number)
> 
> | id |
> id := LargePositiveInteger.
> 1 to: (16777215 + 50) do: [:each |
>   id := id digitAdd: 1].
> id.
> 
> Given the comment it should overflow and the value be 50? This is not what 
> the result is. So shall the truncation be added again or at least the comment 
> be updated? The code will go from LargePositiveInteger to SmallInteger when 
> overflowing from three to four bytes but luckily
> 
>>> #value
>   ...
>   replaceFrom: 1 to: 3with: self class counterNext startingAt: 1
> 
> will even work when counterNext returns a SmallInteger. But given the old 
> code and the comment in the new code this is does not seem to function as 
> intended?
> 
> kind regards
>   holger
> 
> 
> PS: The other part is that >>#newCounter doesn't seem to be ever executed. On 
> first load >>#initialize will call >>#reset and on >>#shutDown: calls reset. 
> So the code to "randomize" the initial counter doesn't seem to work.
> 
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail


[Pharo-users] Mongo-BSON OID LargePositiveInteger increase

2016-05-30 Thread Holger Freyther
Hi,

I tried to reach the author for several weeks but he doesn't seem to respond so 
I am trying to reach a wider audience to either confirm my suspicion or to be 
corrected. In
http://smalltalkhub.com/#!/~MongoTalkTeam/mongotalk/diff/Mongo-BSON-HenrikSperreJohansen.43
 

the following change is done:

+   "digitAdd: wraps around to 0 when result would overflow"
+   ^ counter := counter 
+   ifNil:[self newCounter] 
+   ifNotNil:[counter digitAdd: 1]!
-   ^ counter := (counter + 1) \\ 16rFF!

The old code has overflow checking, the new code makes a statement I don't 
think is true. counter is LargePositiveInteger new: 3 to use three bytes. So 
given the above code and the experiment (yes I could just add a bigger number)

| id |
id := LargePositiveInteger.
1 to: (16777215 + 50) do: [:each |
id := id digitAdd: 1].
id.

Given the comment it should overflow and the value be 50? This is not what the 
result is. So shall the truncation be added again or at least the comment be 
updated? The code will go from LargePositiveInteger to SmallInteger when 
overflowing from three to four bytes but luckily

>>#value
...
replaceFrom: 1 to: 3with: self class counterNext startingAt: 1

will even work when counterNext returns a SmallInteger. But given the old code 
and the comment in the new code this is does not seem to function as intended?

kind regards
holger


PS: The other part is that >>#newCounter doesn't seem to be ever executed. On 
first load >>#initialize will call >>#reset and on >>#shutDown: calls reset. So 
the code to "randomize" the initial counter doesn't seem to work.