On 05.10.2011 01:59, Nicolas Cellier wrote:
2011/10/5 Michael Roberts<mike@mjr104.co.uk>:
I find the thread a bit confused in the sense
Dictionary *new* at: x ifAbsentPut: y
does not make sense (is academic), because the new dictionary will never have x as a key. So why profile it and use that as reasoning?...
whereas
myDict at:x ifAbsentPut: y
is more interesting.
I guess in some examples, like when you want to serialize an object graph with as many nodes as branches (no or few sharing), then the case of absence is dominating. Knowing that Mariano is working on Fuel, i guess that could have biased his judgement.
In my experience i am biased by the use of the message in the systems that already use it heavily...however it always feels that the use of ifAbsentXX is intentionally deferring some code because it expresses exceptional cases in the application logic. Especially if the block makes an object you don't want to be doing that every time and discarding the result irrespective of the presence of the key. It does not have to be expensive to make, just not good style near any kind of loop.
I know some people that actually don't like the ifAbsentPut: [] variant. and prefer to be explicit, of the form
(myDict includesKey: x) ifFalse: [ myDict at: x put: y]
Both codes are valid. But note that at:ifAbsentPut: will return the value, or the newly put value. So the exact equivalent is ((myDict includesKey: x) ifTrue: [myDict at: x] ifFalse: [myDict at: x put: y])
However, maybe Mariano is searching for tight inner loop optimization (we can only guess, because he didn't tell, he should have). In this case, using ifFalse: is good: because it is inlined by Compiler, it avoids a BlockClosure creation. But above code will cost two lookup in both branch, so it will be pretty bad too, depending on hash evaluation cost and collision rate, in a majority of cases, worse than block creation time. I would suggest to Mariano to implement in the Dictionary class of his choice the single lookup, block free creation code:
at: key ifAbsentPutValue: aValue "Answer the value associated with the key. if key isn't found, associate it first with aValue."
| index | ^((array at: (index := self scanFor: key)) ifNil: [ array at: index put: (key -> aValue). aValue ] ifNotNil: [:value | value]
But, please, please, Mariano, don't change #at:ifAbsentPut: As other said, conditional execution is useful both for speed and because in Object world, the state can change. aBagOfKeys do: [:key | myDict at: key ifAbsentPut: (myStream next)]. aBagOfKeys do: [:key | myDict at: key ifAbsentPut: [myStream next]]. won't behave the same if aBagOfKeys has duplicates, will they ?
Last thing, #at:ifAbsentPut: is absolutely decoupled from Object>>#value, it does rely on Object>>#value So don't throw #at:ifAbsentPut: because you don't like Object>>#value. If you don't like it, don't use it, use a BlockClosure instead.
Anyway, thanks to Mariano for these questions, they were biased, but not silly, no question is :)
Nicolas
You could make at:ifAbsentPut: do single lookup, at the cost of using cull: (with the scanned for index) in at:ifAbsent: . Sort of restricts what you can pass to at:ifAbsent: though, (not that I found any not using a block in a quick scan of senders, nor did my image crash) and the runtime really didn't improve that much. | dict | dict := Dictionary new: 100000. [1 to: 100000 do: [: i | "1 scan (new)" "2 scans (old)" dict at: i ifAbsentPut: i ]] timeToRun 34 34 48 38 82 34 58 35 33 42 76 57 56 40 40 57 120 48 This is numbers in a presized dictionary though, so the collision rate might not be representative... Let me check that with points instead, where the hash computation/lookup cost is higher... | dict pts| dict := Dictionary new: 100000. pts := (1 to: 100000) collect: [:i | i@i]. [1 to: pts size do: [: ix | "1 scan (new)" "2 scans (old)" dict at: (pts at: ix) ifAbsentPut: ix ]] timeToRun 96 89 89 89 91 143 92 89 96 132 127 144 126 139 126 133 129 127 134 Still not worth it to restrict at:ifAbsent: imho. Cheers, Henry