On Sat, 17 Dec 2011, Nicolas Cellier wrote:
2011/12/17 Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com>:
2011/12/17 Mariano Martinez Peck <marianopeck@gmail.com>:
Hi Levente. I take a deeper look to see why LargeIdentityDictionary is not working as expected for us.
The way we are putting objects in the Dict is always this way:
registerIndexesOn: aDictionary
   self objects do: [ :instance | aDictionary at: instance put: aDictionary size + 1 ].
So?keys are the objects of the graphs and the values are the internal position in the serialization. However, when I run a serialization it looks like if the follow happens that when we are doing:
encodeReferenceTo: anObject
   indexCluster       serialize: (objectsIndexes at: anObject)       on: stream
(objectsIndexes at: anObject) == anObject  -> true
so (objectsIndexes at: anObject) is answering "self" rather than the number (which was the dict size + 1 at the serialization moment).
Something interesting is that most tests seem to pass and the problem is with benchmarks (where graphs are much bigger), so maybe it could be something related to that. For example, the following benchmark for bitmaps is failing:
        numbers := OrderedCollection new.    bitmaps := OrderedCollection new.    numbers addAll: ( 1 << 30 to:( ( 1 << 30) + 10000) ) asArray.    numbers do: [:each | bitmaps add: (Bitmap with: each with: each + 1) ].    ^ bitmaps
it happens the mentioned problem when trying to serialize the collection 'bitmaps'.
I tried to reproduce it outside Fuel and I think I succeeded (not sure). The following does work with IdentityDictionary but not with LargeIdentityDictionary. Basically I am simulating the way we use this dict in Fuel:
testLargeIdentityDictionary
   | numbers bitmaps dict |
"Create some bitmaps"
   numbers := OrderedCollection new.    bitmaps := OrderedCollection new.    numbers addAll: ( 1 << 30 to:( ( 1 << 30) + 10000) ) asArray.    numbers do: [:each | bitmaps add: (Bitmap with: each with: each + 1) ].
"Put them in a dictionary using dict as as key" Â Â Â dict := LargeIdentityDictionary new. Â Â Â bitmaps do: [:each | Â Â Â Â Â Â Â dict at: each put: dict size + 1. Â Â Â Â Â Â ].
"Check they are correct" Â Â Â bitmaps doWithIndex: [:each :index | self assert: index == (dict at: each)]
Of course that test works with IdentityDictionary.
Can't you debug it ?
It took 2 minutes to find this sentence at line 19 of LargeDictionary>>at:put:
(values at: hash) at: newIndex put: key
Can you replace key with value and retry the tests ?
Nice find, thanks. :) Levente
Nicolas
Thanks for any idea.
On Sat, Dec 17, 2011 at 1:50 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
On Dec 16, 2011, at 3:28 PM, Levente Uzonyi wrote:
Cool. One more thing: in Squeak the method using primitive 132 directly was renamed to #instVarsInclude:, so now #pointsTo: works as expected. If this was also added to Pharo, then the #pointsTo: sends should be changed to #instVarsInclude:, otherwise Array can be reported as included even if it wasn't added. I'll upload my LargeIdentityDictionary implementation to the same place this evening, since it's still 2-3 factor faster than other solutionts and there seem to be demand for it.
Levente
"in Squeak the method using primitive 132 directly was renamed to #instVarsInclude:, so now #pointsTo: works as expected." I do not get the following. Indeed pointTo: looks like instVarsInclude: now I do not understand the rest of your paragraph.
Stef
-- Mariano http://marianopeck.wordpress.com