WeakValueDictionary: shouldn't be "absent" after GC?
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 [image: Inline image 1] Maybe checking for emptiness first is needed before going for values include: Phil On Mon, Feb 20, 2017 at 11:18 PM, Martin Dias <tinchodias@gmail.com> wrote:
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
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, phil@highoctane.be <phil@highoctane.be> wrote:
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
[image: Inline image 1]
Maybe checking for emptiness first is needed before going for values include:
Phil
On Mon, Feb 20, 2017 at 11:18 PM, Martin Dias <tinchodias@gmail.com> wrote:
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
Hi. 2017-02-21 9:44 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
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.
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 <dionisiydk@gmail.com> wrote:
Hi.
2017-02-21 9:44 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
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.
For me what you describe is implementation details and users should not think about it at all.
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. 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.
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 <guillermopolito@gmail.com>:
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.
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 <dionisiydk@gmail.com> wrote:
2017-02-21 13:07 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
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.
So you think my suggestion for implementation will lead to some tradeoff? What exactly? (if yes).
Ok. Question was from Martin. What behaviour we want from WeakValueDictionary for garbaged values? 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)
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: set := WeakSet with: Object new. Smalltalk garbageCollect. set includes: nil "=>false" But after writing it I realized how much WeakSet is inconsistent: set := WeakSet with: Object new. Smalltalk garbageCollect. set isEmpty. "=>false" set size. "=>1". set anyOne. "=> nil" set includes: nil. "=>false" set collect: #yourself. "=>a WeakSet()" We definitely have something to improve here. 2017-02-21 13:36 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
Everything has a tradeoff. I do not understand the question.
On Tue, Feb 21, 2017 at 1:19 PM, Denis Kudriashov <dionisiydk@gmail.com> wrote:
2017-02-21 13:07 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
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.
So you think my suggestion for implementation will lead to some tradeoff? What exactly? (if yes).
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 <tinchodias@gmail.com> 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. 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) ]
Bah, better: morphAt: key cache ifNil: [ cache := WeakValueDictionary new ]. ^ cache at: key ifAbsentPut: [ self newMorph: key ]
:-)
MartÃn
On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <tinchodias@gmail.com> 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.
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 Wed, Feb 22, 2017 at 8:52 PM, Guillermo Polito <guillermopolito@gmail.com> wrote:
On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <tinchodias@gmail.com> 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 <btc@openinworld.com> wrote:
On Wed, Feb 22, 2017 at 8:52 PM, Guillermo Polito <guillermopolito@gmail.com> wrote:
On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <tinchodias@gmail.com>
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
On Wed, Feb 22, 2017 at 9:52 AM, Guillermo Polito <guillermopolito@gmail.com
wrote:
On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <tinchodias@gmail.com> 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.
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. https://pharo.fogbugz.com/f/cases/19746/WeakValueDictionary-has-counter-intu...
In any case, we could see how to improve EphemeronRegistry or even implement an EphemericDictionary that would replace the WeakValueDictionary.
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
Thanks Martin :) On Thu, Feb 23, 2017 at 6:18 AM, Martin Dias <tinchodias@gmail.com> wrote:
On Wed, Feb 22, 2017 at 9:52 AM, Guillermo Polito < guillermopolito@gmail.com> wrote:
On Wed, Feb 22, 2017 at 12:57 AM, Martin Dias <tinchodias@gmail.com> 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.
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.
https://pharo.fogbugz.com/f/cases/19746/WeakValueDictionary-has- counter-intuitive-behavior
In any case, we could see how to improve EphemeronRegistry or even implement an EphemericDictionary that would replace the WeakValueDictionary.
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
participants (5)
-
Ben Coman -
Denis Kudriashov -
Guillermo Polito -
Martin Dias -
phil＠highoctane.be