WeakValueDictionary: shouldn't be "absent" after GC?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

WeakValueDictionary: shouldn't be "absent" after GC?

tinchodias
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
Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

philippeback
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

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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Guillermo Polito
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:
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

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 <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Denis Kudriashov
Hi.

2017-02-21 9:44 GMT+01:00 Guillermo Polito <[hidden email]>:
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.
Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Guillermo Polito


On Tue, Feb 21, 2017 at 10:37 AM, Denis Kudriashov <[hidden email]> wrote:
Hi.

2017-02-21 9:44 GMT+01:00 Guillermo Polito <[hidden email]>:
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.
Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Denis Kudriashov

2017-02-21 13:07 GMT+01:00 Guillermo Polito <[hidden email]>:
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).
Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Guillermo Polito
Everything has a tradeoff. I do not understand the question.

On Tue, Feb 21, 2017 at 1:19 PM, Denis Kudriashov <[hidden email]> wrote:

2017-02-21 13:07 GMT+01:00 Guillermo Polito <[hidden email]>:
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).

Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Denis Kudriashov
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 <[hidden email]>:
Everything has a tradeoff. I do not understand the question.

On Tue, Feb 21, 2017 at 1:19 PM, Denis Kudriashov <[hidden email]> wrote:

2017-02-21 13:07 GMT+01:00 Guillermo Polito <[hidden email]>:
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).


Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

tinchodias
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
Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

tinchodias

On Tue, Feb 21, 2017 at 8:57 PM, 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. 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

Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Guillermo Polito
In reply to this post by tinchodias


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.



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

Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Ben Coman
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
>
>

Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Guillermo Polito
I trust your native english advice :)

On Wed, Feb 22, 2017 at 2:11 PM, Ben Coman <[hidden email]> wrote:
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
>
>


Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

tinchodias
In reply to this post by Guillermo Polito


On Wed, Feb 22, 2017 at 9:52 AM, 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.


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.



 
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


Reply | Threaded
Open this post in threaded view
|

Re: WeakValueDictionary: shouldn't be "absent" after GC?

Guillermo Polito
Thanks Martin :)

On Thu, Feb 23, 2017 at 6:18 AM, Martin Dias <[hidden email]> wrote:


On Wed, Feb 22, 2017 at 9:52 AM, 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.


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.



 
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