Hi. The answer of: | dictionary | dictionary := WeakValueDictionary with: 'hello' -> 'world' copy. Smalltalk garbageCollect. { dictionary values includes: nil. dictionary at: 'hello'. dictionary at: 'hello' ifAbsent: [ 'absent' ]. dictionary at: 'hello' ifAbsentPut: [ 'put' ]. } is: #(true nil nil nil) Bug or feature? shouldn't the value be considered as absent instead of present (and nil). cheers, Martín |
This makes sense, no? | dictionary | dictionary := WeakValueDictionary with: 'hello' -> nil copy. "Smalltalk garbageCollect." r:={ dictionary values includes: nil. dictionary at: 'hello'. dictionary at: 'hello' ifAbsent: [ 'absent' ]. dictionary at: 'hello' ifAbsentPut: [ 'put' ]. }. r and I get this anyway when GCing and puttting #x as value Maybe checking for emptiness first is needed before going for values include: Phil On Mon, Feb 20, 2017 at 11:18 PM, Martin Dias <[hidden email]> wrote:
|
Hi Dr. Dias, a WeakValueDictionary get's its values collected if they are not referenced strongly. However, the association remains there. This means that when you do: dictionary := WeakValueDictionary with: 'hello' -> nil copy. You'll have something like this: WeakValueDictionary { WeakValueAssociation { key: 'hello', value: yourObject } } Once you garbage collect, the value is holding yourObject weakly, so you'll have: WeakValueDictionary { WeakValueAssociation { key: 'hello', value: nil } } But the WeakValueAssociation will still be there. Apparently, what you want to do is to automatically clean your dictionary. Checking a Pharo60371, I see that WeakValueDictionaries do not implement this... But WeakKeyDictionary does. Take for example a look at WeakKeyDictionary>>finalizeValues. If you subscribe the WeakKeyDictionary instance to the WeakRegistry, the weak registry will send finalize values to your weak key dictionary and they will eliminate the expired associations. However, #finalizeValues is not implemented in WeakValueDictionary. Maybe we should? Or maybe you should try with Ephemerons. Guille BTW why are you using a copy of nil? you can test it with a plain new Object (i.e., Object new). It took me a couple of extra seconds to realize what you were doing. On Tue, Feb 21, 2017 at 7:57 AM, [hidden email] <[hidden email]> wrote:
|
Hi. 2017-02-21 9:44 GMT+01:00 Guillermo Polito <[hidden email]>:
For me what you describe is implementation details and users should not think about it at all. I agree ifAbsent~ messages should work with collected values by "absent branch". It is intuitive. To support it correctly we can use approach of Set which uses #asSetElement to store items. Then we can easily distinguish nil from absent value. Real nil value will happen only by garbage collection. But I am wondering WeakSet not uses SetElement approach directly and instead it uses special flag object. Maybe it should be fixed too. |
On Tue, Feb 21, 2017 at 10:37 AM, Denis Kudriashov <[hidden email]> wrote:
Of course it is an implementation detail. I'm just describing how it works (and thus, how it does not work). I never said it was a good or bad approach.
But in any case, whatever the alternative you give users, they must understand the tradeoff. If all Weak*Dictionaries were subscribed by default to the weak registry for cleanup, that would provoke a lot of overhead during garbage collection. Magic is good in some cases, but when you're dealing with low level stuff explicitly (like instantiating a Weak*Dictionary) I prefer to control some of that magic explicitly. |
2017-02-21 13:07 GMT+01:00 Guillermo Polito <[hidden email]>:
So you think my suggestion for implementation will lead to some tradeoff? What exactly? (if yes). |
Everything has a tradeoff. I do not understand the question. On Tue, Feb 21, 2017 at 1:19 PM, Denis Kudriashov <[hidden email]> wrote:
|
Ok. Question was from Martin. What behaviour we want from WeakValueDictionary for garbaged values? dictionary := WeakValueDictionary with: 'hello' -> 'world' copy. My opinion is: it should work in same way when 'hello' item is absent. It is intuitive for me and It would be consistent to WeakSet behaviour:
But after writing it I realized how much WeakSet is inconsistent:
We definitely have something to improve here. 2017-02-21 13:36 GMT+01:00 Guillermo Polito <[hidden email]>:
|
Hi all Phil: I'm sorry, didn't get what you point with the code snippet. The copy of the Symbol is the same instance, that I think it's nto GCed. Guille: Yes, I knew the internals of the WeakValueDictionary but asked about the API. What I don't know is about using WeakRegistry and Ephemerons. I'll check WeakKeyDictionary. I understand the idea of "explicit control of magic", but not sure if I understand concretely. Do you mean to implement something like WeakValueDictionary>>register which will not be executed by #initialize, so the user can decide to register. Am I right? Denis: +1 WeakSet behavior is really confusing! In thte case of WeakValueDictionary I didn't check for inconsistencies but was only annoyed but it's behavior. Guille and Denis: Instead of using SetElement or cleaning up the empty WeakValueAssociations, what I first thought is to internally consider an association with nil as absent. I mean, modify or override some methods soem when there is an WeakValueAssociation with value == nil it considers it's absent. Concretely, I'd expect: | dictionary | dictionary := WeakValueDictionary with: 'hello' -> 'world' copy. Smalltalk garbageCollect. { dictionary values includes: nil. ---> false dictionary at: 'hello'. ---> NotFound signal dictionary at: 'hello' ifAbsent: [ 'absent' ]. ---> 'absent' dictionary at: 'hello' ifAbsentPut: [ 'put' ]. ---> 'put' } It's better if I give some context. Look this simplified version of my use case: MyUI>> morphAt: key cache ifNil: [ cache := WeakValueDictionary new ]. ^ cache
at: key
ifPresent: [:cachedValueOrNil |
cachedValueOrNil
ifNotNil: [ cachedValueOrNil ]
ifNil: [ cache at: entryReference put: (self newMorph: key) ] ]
ifAbsent: [ cache at: entryReference put: (self newMorph: key) ] I'd like to only send #newMorph: in ifAbsent: and to avoid the ifNotNil:ifNil: Like this: morphAt: key cache ifNil: [ cache := WeakValueDictionary new ]. ^ cache
at: key
ifAbsent: [ cache at: entryReference put: (self newMorph: key) ] :-) Martín |
On Tue, Feb 21, 2017 at 8:57 PM, Martin Dias <[hidden email]> wrote:
Bah, better: morphAt: key cache ifNil: [ cache := WeakValueDictionary new ]. ^ cache
at: key
ifAbsentPut: [ self newMorph: key ]
|
In reply to this post by tinchodias
On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <[hidden email]> wrote:
Ah, ok, I only understood that you did not get why it was like that. :)
This is just needed to do the cleanup of the expired associations.
This is a completely different thing. There is an EphemeronRegistry in the image, but it does not propose at:/at:ifAbsent: messages. It just holds Ephemerons.
If you need to hold weakly the values, then this is not what you're looking for, isn't it?
Yep. I would even call it: registerForCleanup Like that it is explicit that you're doing it, and that it may incur into some performance degradation of the entire system.
Well, the idea of the SetElement is that you may want to have `nil` as a valid value in your dictionary. Otherwise, you cannot do: WeakValueDictionary new at: 'key' put: nil; at: 'key' --> error??? We can discuss if it is useful or not, but as with sets, before people used to check if the inserted element was nil beforehand to have the expected behavior...
I agree that we may review the API. At least #at:, #at:ifAbsent: and #at:ifAbsentPut: should behave similarly. In any case, we could see how to improve EphemeronRegistry or even implement an EphemericDictionary that would replace the WeakValueDictionary.
|
On Wed, Feb 22, 2017 at 8:52 PM, Guillermo Polito
<[hidden email]> wrote: > > > On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <[hidden email]> wrote: >> >> Hi all >> >> Phil: >> I'm sorry, didn't get what you point with the code snippet. The copy of >> the Symbol is the same instance, that I think it's nto GCed. >> >> Guille: >> Yes, I knew the internals of the WeakValueDictionary but asked about the >> API. > > > Ah, ok, I only understood that you did not get why it was like that. :) > >> >> What I don't know is about using WeakRegistry > > > This is just needed to do the cleanup of the expired associations. > >> >> and Ephemerons. > > > This is a completely different thing. There is an EphemeronRegistry in the > image, but it does not propose at:/at:ifAbsent: messages. It just holds > Ephemerons. > >> I'll check WeakKeyDictionary. > > > If you need to hold weakly the values, then this is not what you're looking > for, isn't it? > >> >> >> I understand the idea of "explicit control of magic", but not sure if I >> understand concretely. Do you mean to implement something like >> WeakValueDictionary>>register which will not be executed by #initialize, so >> the user can decide to register. Am I right? > > > Yep. I would even call it: > > registerForCleanup > > Like that it is explicit that you're doing it, and that it may incur into > some performance degradation of the entire system. > >> >> >> Denis: >> +1 WeakSet behavior is really confusing! In thte case of >> WeakValueDictionary I didn't check for inconsistencies but was only annoyed >> but it's behavior. >> >> Guille and Denis: >> Instead of using SetElement or cleaning up the empty >> WeakValueAssociations, what I first thought is to internally consider an >> association with nil as absent. I mean, modify or override some methods soem >> when there is an WeakValueAssociation with value == nil it considers it's >> absent. Concretely, I'd expect: > > > Well, the idea of the SetElement is that you may want to have `nil` as a > valid value in your dictionary. Otherwise, you cannot do: > > WeakValueDictionary new > at: 'key' put: nil; > at: 'key' --> error??? > > We can discuss if it is useful or not, but as with sets, before people used > to check if the inserted element was nil beforehand to have the expected > behavior... > >> >> | dictionary | >> dictionary := WeakValueDictionary with: 'hello' -> 'world' copy. >> Smalltalk garbageCollect. >> { >> dictionary values includes: nil. ---> false >> dictionary at: 'hello'. ---> NotFound signal >> dictionary at: 'hello' ifAbsent: [ 'absent' ]. ---> 'absent' >> dictionary at: 'hello' ifAbsentPut: [ 'put' ]. ---> 'put' >> } > > > I agree that we may review the API. At least #at:, #at:ifAbsent: and > #at:ifAbsentPut: should behave similarly. > > In any case, we could see how to improve EphemeronRegistry or even implement > an EphemericDictionary that would replace the WeakValueDictionary. btw, Would EmphemeralDictionary be an acceptable name? It has a nicer sound to it. cheers -ben > >> >> >> It's better if I give some context. Look this simplified version of my use >> case: >> >> MyUI>> >> morphAt: key >> cache ifNil: [ cache := WeakValueDictionary new ]. >> ^ cache at: key ifPresent: [:cachedValueOrNil | cachedValueOrNil >> ifNotNil: [ cachedValueOrNil ] ifNil: [ cache at: entryReference put: (self >> newMorph: key) ] ] ifAbsent: [ cache at: entryReference put: (self newMorph: >> key) ] >> >> I'd like to only send #newMorph: in ifAbsent: and to avoid the >> ifNotNil:ifNil: >> Like this: >> >> morphAt: key >> cache ifNil: [ cache := WeakValueDictionary new ]. >> ^ cache at: key ifAbsent: [ cache at: entryReference put: (self newMorph: >> key) ] >> >> :-) >> >> Martín > > |
I trust your native english advice :) On Wed, Feb 22, 2017 at 2:11 PM, Ben Coman <[hidden email]> wrote:
|
In reply to this post by Guillermo Polito
On Wed, Feb 22, 2017 at 9:52 AM, Guillermo Polito <[hidden email]> wrote:
OK, I wrote several tests for the expected behavior and fixed WeakValueDictionary. Anyway, not sure if it's convenient to integrate it in Pharo 6 or in any version... maybe it's just OK to check for nils at user-level as currently, and do not change it. I had to use SetElement else tests didn't pass.
|
Thanks Martin :) On Thu, Feb 23, 2017 at 6:18 AM, Martin Dias <[hidden email]> wrote:
|
Free forum by Nabble | Edit this page |