The Trunk: System-cwp.660.mcz

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

The Trunk: System-cwp.660.mcz

commits-2
Colin Putney uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-cwp.660.mcz

==================== Summary ====================

Name: System-cwp.660
Author: cwp
Time: 8 January 2014, 9:02:53.874 pm
UUID: 3fb6a88d-86cf-4625-b7b5-e59a476195b3
Ancestors: System-cwp.659

Add a test for 0 in #allObjectsDo:, as it's possible for the sentinel object not to be enumerated.

=============== Diff against System-cwp.659 ===============

Item was changed:
  ----- Method: SystemNavigation>>allObjectsDo: (in category 'query') -----
  allObjectsDo: aBlock
  "Evaluate the argument, aBlock, for each object in the system
  excluding SmallIntegers. With closures, this needs to use an end
  marker (lastObject) since activation of the block will create new
  contexts and cause an infinite loop."
  | object lastObject |
  object := self someObject.
  lastObject := Object new.
+ [lastObject == object | object == 0]
- [lastObject == object]
  whileFalse: [aBlock value: object.
  object := object nextObject]!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Levente Uzonyi-2
On Thu, 9 Jan 2014, [hidden email] wrote:

> Colin Putney uploaded a new version of System to project The Trunk:
> http://source.squeak.org/trunk/System-cwp.660.mcz
>
> ==================== Summary ====================
>
> Name: System-cwp.660
> Author: cwp
> Time: 8 January 2014, 9:02:53.874 pm
> UUID: 3fb6a88d-86cf-4625-b7b5-e59a476195b3
> Ancestors: System-cwp.659
>
> Add a test for 0 in #allObjectsDo:, as it's possible for the sentinel object not to be enumerated.

How can that happen?


Levente

>
> =============== Diff against System-cwp.659 ===============
>
> Item was changed:
>  ----- Method: SystemNavigation>>allObjectsDo: (in category 'query') -----
>  allObjectsDo: aBlock
>   "Evaluate the argument, aBlock, for each object in the system
>   excluding SmallIntegers. With closures, this needs to use an end
>   marker (lastObject) since activation of the block will create new
>   contexts and cause an infinite loop."
>   | object lastObject |
>   object := self someObject.
>   lastObject := Object new.
> + [lastObject == object | object == 0]
> - [lastObject == object]
>   whileFalse: [aBlock value: object.
>   object := object nextObject]!
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Levente Uzonyi-2
In reply to this post by commits-2
On Thu, 9 Jan 2014, [hidden email] wrote:

> Colin Putney uploaded a new version of System to project The Trunk:
> http://source.squeak.org/trunk/System-cwp.660.mcz
>
> ==================== Summary ====================
>
> Name: System-cwp.660
> Author: cwp
> Time: 8 January 2014, 9:02:53.874 pm
> UUID: 3fb6a88d-86cf-4625-b7b5-e59a476195b3
> Ancestors: System-cwp.659
>
> Add a test for 0 in #allObjectsDo:, as it's possible for the sentinel object not to be enumerated.
>
> =============== Diff against System-cwp.659 ===============
>
> Item was changed:
>  ----- Method: SystemNavigation>>allObjectsDo: (in category 'query') -----
>  allObjectsDo: aBlock
>   "Evaluate the argument, aBlock, for each object in the system
>   excluding SmallIntegers. With closures, this needs to use an end
>   marker (lastObject) since activation of the block will create new
>   contexts and cause an infinite loop."
>   | object lastObject |
>   object := self someObject.
>   lastObject := Object new.
> + [lastObject == object | object == 0]

Btw, this won't work, because this expression is evaluated as

  ((lastObject == object) | object) == 0

which should raise an error, because most objects don't understand #|.
Also, I prefer the lazy #or: over #|, because it's faster.


Levente

> - [lastObject == object]
>   whileFalse: [aBlock value: object.
>   object := object nextObject]!
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Bert Freudenberg
In reply to this post by Levente Uzonyi-2

On 09.01.2014, at 07:55, Levente Uzonyi <[hidden email]> wrote:

> On Thu, 9 Jan 2014, [hidden email] wrote:
>
>> Colin Putney uploaded a new version of System to project The Trunk:
>> http://source.squeak.org/trunk/System-cwp.660.mcz
>>
>> ==================== Summary ====================
>>
>> Name: System-cwp.660
>> Author: cwp
>> Time: 8 January 2014, 9:02:53.874 pm
>> UUID: 3fb6a88d-86cf-4625-b7b5-e59a476195b3
>> Ancestors: System-cwp.659
>>
>> Add a test for 0 in #allObjectsDo:, as it's possible for the sentinel object not to be enumerated.
>
> How can that happen?
>
>
> Levente
Maybe if inside aBlock you #become: the current object to one newer than lastObject?

Of course, in that case you wouldn't enumerate all objects, so you shouldn't do that. If that's the case you want to solve, then reaching 0 should raise an error. Something like

  [lastObject == object]
                whileFalse: [
                        aBlock value: object.
                        (object := object nextObject) == 0 ifTrue: [
                                ^self error: 'should not happen']]!


which would be better than an infinite loop.

- Bert -


>
>>
>> =============== Diff against System-cwp.659 ===============
>>
>> Item was changed:
>> ----- Method: SystemNavigation>>allObjectsDo: (in category 'query') -----
>> allObjectsDo: aBlock
>> "Evaluate the argument, aBlock, for each object in the system
>> excluding SmallIntegers. With closures, this needs to use an end
>> marker (lastObject) since activation of the block will create new
>> contexts and cause an infinite loop."
>> | object lastObject |
>> object := self someObject.
>> lastObject := Object new.
>> + [lastObject == object | object == 0]
>> - [lastObject == object]
>> whileFalse: [aBlock value: object.
>> object := object nextObject]!
>>
>>
>>
>



smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Levente Uzonyi-2



On Thu, Jan 9, 2014 at 2:01 AM, Levente Uzonyi <[hidden email]> wrote:
 
Btw, this won't work, because this expression is evaluated as

        ((lastObject == object) | object) == 0

which should raise an error, because most objects don't understand #|.
Also, I prefer the lazy #or: over #|, because it's faster.

You're right, it's a bug, but it does work, accidentally. It ends up going like this:

1. lastObject == object -> aBoolean
2. aBoolean | object -> true or object
3a. true == 0 -> false
3b. object == 0 -> true or false

For all objects except the sentinel object, we end up at 3b, and since #nextObject answers 0 when the receiver is the last object in the heap, we end up with the correct semantics. 

However, it defeats the purpose of the sentinel object, which is to avoid an infinite loop caused by new block contexts being allocated when the block is executed. The reason I needed this change in the first place is that I was getting an error because SmallInteger shouldn't implement #nextObject. So somehow we were getting to the end of the heap without encountering the sentinel. 

So I have two questions:

1) Why isn't the sentinel object enumerated in a shrunken image?
2) Why don't we get an infinite loop when we don't check for the sentinel?

Colin


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Bert Freudenberg



On Thu, Jan 9, 2014 at 5:14 AM, Bert Freudenberg <[hidden email]> wrote:
 
 
Maybe if inside aBlock you #become: the current object to one newer than lastObject?

Of course, in that case you wouldn't enumerate all objects, so you shouldn't do that. If that's the case you want to solve, then reaching 0 should raise an error

No, the case I wanted to solve was that reaching zero was raising an error. :-)

SmallInteger>>nextObject sends #shouldNotImplement.

Colin 



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Levente Uzonyi-2



On Thu, Jan 9, 2014 at 2:01 AM, Levente Uzonyi <[hidden email]> wrote:
 
Also, I prefer the lazy #or: over #|, because it's faster.

Normally, yes, but in this case, I think #| is actually faster. Consider the two versions (without the bug):

a)  (lastObject == object) | (object == 0)

b) (lastObject == object) or: [object == 0]

Lazy evaluation doesn't help here, because the first comparison will almost never be true. On the other hand, #== is very fast, because it's a bytecode, whereas #or: involves an extra message send and creating a stack frame to execute the block.

Colin



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Nicolas Cellier



2014/1/9 Colin Putney <[hidden email]>



On Thu, Jan 9, 2014 at 2:01 AM, Levente Uzonyi <[hidden email]> wrote:
 
Also, I prefer the lazy #or: over #|, because it's faster.

Normally, yes, but in this case, I think #| is actually faster. Consider the two versions (without the bug):

a)  (lastObject == object) | (object == 0)

b) (lastObject == object) or: [object == 0]

Lazy evaluation doesn't help here, because the first comparison will almost never be true. On the other hand, #== is very fast, because it's a bytecode, whereas #or: involves an extra message send and creating a stack frame to execute the block.

Colin

err, yes, lazyness don't pay, but no, #or: is inlined by compiler as a conditional jump...
It's the contrary, #| is a message send.



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Eliot Miranda-2
In reply to this post by Colin Putney-3



On Thu, Jan 9, 2014 at 6:06 AM, Colin Putney <[hidden email]> wrote:



On Thu, Jan 9, 2014 at 2:01 AM, Levente Uzonyi <[hidden email]> wrote:
 
Btw, this won't work, because this expression is evaluated as

        ((lastObject == object) | object) == 0

which should raise an error, because most objects don't understand #|.
Also, I prefer the lazy #or: over #|, because it's faster.

You're right, it's a bug, but it does work, accidentally. It ends up going like this:

1. lastObject == object -> aBoolean
2. aBoolean | object -> true or object
3a. true == 0 -> false
3b. object == 0 -> true or false

For all objects except the sentinel object, we end up at 3b, and since #nextObject answers 0 when the receiver is the last object in the heap, we end up with the correct semantics. 

However, it defeats the purpose of the sentinel object, which is to avoid an infinite loop caused by new block contexts being allocated when the block is executed. The reason I needed this change in the first place is that I was getting an error because SmallInteger shouldn't implement #nextObject. So somehow we were getting to the end of the heap without encountering the sentinel. 

So I have two questions:

1) Why isn't the sentinel object enumerated in a shrunken image?

send me an image poised at the right point and I'll take a look...
 
2) Why don't we get an infinite loop when we don't check for the sentinel?

Colin






--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Eliot Miranda-2
In reply to this post by Colin Putney-3



On Thu, Jan 9, 2014 at 6:35 AM, Colin Putney <[hidden email]> wrote:



On Thu, Jan 9, 2014 at 2:01 AM, Levente Uzonyi <[hidden email]> wrote:
 
Also, I prefer the lazy #or: over #|, because it's faster.

Normally, yes, but in this case, I think #| is actually faster. Consider the two versions (without the bug):

a)  (lastObject == object) | (object == 0)

b) (lastObject == object) or: [object == 0]

Lazy evaluation doesn't help here, because the first comparison will almost never be true. On the other hand, #== is very fast, because it's a bytecode, whereas #or: involves an extra message send and creating a stack frame to execute the block.

It's the other way around.  #or: is inlined into jump bytecodes. #| is a real send.
 

Colin







--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Eliot Miranda-2



On Fri, Jan 10, 2014 at 5:32 PM, Eliot Miranda <[hidden email]> wrote:
 
1) Why isn't the sentinel object enumerated in a shrunken image?

send me an image poised at the right point and I'll take a look...

While working on creating this image, I noticed that Bert was right. There is a #becomeForward: happening inside the block that's passed into #allObjectsDo:. It's because #allObjectsDo: is called from #obsoleteBehaviors, which sends #isBehavior to all the objects. If the image happens to contain an instance of MCInfoProxy, the DNU causes it to download the full ancestry and become it. 

However, I don't see why that would cause the sentinel object not to be enumerated. 

Colin


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Levente Uzonyi-2
On Fri, 10 Jan 2014, Colin Putney wrote:

>
>
>
> On Fri, Jan 10, 2014 at 5:32 PM, Eliot Miranda <[hidden email]> wrote:
>  
>       1) Why isn't the sentinel object enumerated in a shrunken image?
>
> send me an image poised at the right point and I'll take a look...
>
>
> While working on creating this image, I noticed that Bert was right. There is a #becomeForward: happening inside the block that's passed into #allObjectsDo:. It's because #allObjectsDo: is called from
> #obsoleteBehaviors, which sends #isBehavior to all the objects. If the image happens to contain an instance of MCInfoProxy, the DNU causes it to download the full ancestry and become it. 
>
> However, I don't see why that would cause the sentinel object not to be enumerated. 
Because #nextObject will be sent to an object which was created after the
sentinel.


Levente

>
> Colin
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Levente Uzonyi-2
On Sat, 11 Jan 2014, Levente Uzonyi wrote:

> On Fri, 10 Jan 2014, Colin Putney wrote:
>
>>
>>
>>
>> On Fri, Jan 10, 2014 at 5:32 PM, Eliot Miranda <[hidden email]>
>> wrote:
>>  
>>       1) Why isn't the sentinel object enumerated in a shrunken image?
>>
>> send me an image poised at the right point and I'll take a look...
>>
>>
>> While working on creating this image, I noticed that Bert was right. There
>> is a #becomeForward: happening inside the block that's passed into
>> #allObjectsDo:. It's because #allObjectsDo: is called from
>> #obsoleteBehaviors, which sends #isBehavior to all the objects. If the
>> image happens to contain an instance of MCInfoProxy, the DNU causes it to
>> download the full ancestry and become it. 
>>
>> However, I don't see why that would cause the sentinel object not to be
>> enumerated. 
>
> Because #nextObject will be sent to an object which was created after the
> sentinel.
Now I see why it wasn't clear. For performance reasons #become* methods
don't move the objects around, they just rewrite the pointers pointing to
them. Here's an example:

a := { $a }.
b := { $b }.
c := { $c }.
self assert: a nextObject == b.
self assert: b nextObject == c.
b become: c.
self assert: a nextObject = #($b).
self assert: b = #($c).
self assert: c = #($b).
self assert: a nextObject == c.
self assert: c nextObject == b.

I think it might help in this case, to ask for the next object before
evaluating the block in the loop of #allObjectsDo:

  [lastObject == object]
  whileFalse: [
  | nextObject |
  nextObject := object nextObject.
  aBlock value: object.
  object := nextObject]


Levente

>
>
> Levente
>
>>
>> Colin
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Levente Uzonyi-2



On Sat, Jan 11, 2014 at 2:53 AM, Levente Uzonyi <[hidden email]> wrote:
 
Because #nextObject will be sent to an object which was created after the sentinel.

/me smacks forehead

Of course! Once the proxy has become the new object, it will receive the #nextObject message inside #allObjectsDo:. That skips a lot of objects, including the sentinel.


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Levente Uzonyi-2



On Sat, Jan 11, 2014 at 3:13 AM, Levente Uzonyi <[hidden email]> wrote:
 

I think it might help in this case, to ask for the next object before evaluating the block in the loop of #allObjectsDo:

        [lastObject == object]
                whileFalse: [
                        | nextObject |
                        nextObject := object nextObject.
                        aBlock value: object.
                        object := nextObject]


Yes, that's definitely an improvement. 

One thing I still don't understand, though, is why we get 0. In theory, we need the sentinel to prevent an infinite loop, but when we skip over it, we still get to the end of the object memory. 

Colin


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Bert Freudenberg

On 11.01.2014, at 17:37, Colin Putney <[hidden email]> wrote:




On Sat, Jan 11, 2014 at 3:13 AM, Levente Uzonyi <[hidden email]> wrote:
 

I think it might help in this case, to ask for the next object before evaluating the block in the loop of #allObjectsDo:

        [lastObject == object]
                whileFalse: [
                        | nextObject |
                        nextObject := object nextObject.
                        aBlock value: object.
                        object := nextObject]


Yes, that's definitely an improvement. 

Great idea! But add a comment about this discussion to explain why we need the extra variable :)

One thing I still don't understand, though, is why we get 0. In theory, we need the sentinel to prevent an infinite loop, but when we skip over it, we still get to the end of the object memory. 

... which is when you got the 0. Or are you asking why we need the sentinel? Because if aBlock keeps allocating objects, we would never reach the end.

- Bert -





smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Florin Mateoc-4
In reply to this post by Levente Uzonyi-2
On 1/11/2014 3:13 AM, Levente Uzonyi wrote:

> On Sat, 11 Jan 2014, Levente Uzonyi wrote:
>
>> On Fri, 10 Jan 2014, Colin Putney wrote:
>>
>>>
>>>
>>>
>>> On Fri, Jan 10, 2014 at 5:32 PM, Eliot Miranda <[hidden email]> wrote:
>>>  
>>>       1) Why isn't the sentinel object enumerated in a shrunken image?
>>>
>>> send me an image poised at the right point and I'll take a look...
>>>
>>>
>>> While working on creating this image, I noticed that Bert was right. There is a #becomeForward: happening inside the
>>> block that's passed into #allObjectsDo:. It's because #allObjectsDo: is called from
>>> #obsoleteBehaviors, which sends #isBehavior to all the objects. If the image happens to contain an instance of
>>> MCInfoProxy, the DNU causes it to download the full ancestry and become it.
>>>
>>> However, I don't see why that would cause the sentinel object not to be enumerated.
>>
>> Because #nextObject will be sent to an object which was created after the sentinel.
>
> Now I see why it wasn't clear. For performance reasons #become* methods
> don't move the objects around, they just rewrite the pointers pointing to them. Here's an example:
>
> a := { $a }.
> b := { $b }.
> c := { $c }.
> self assert: a nextObject == b.
> self assert: b nextObject == c.
> b become: c.
> self assert: a nextObject = #($b).
> self assert: b = #($c).
> self assert: c = #($b).
> self assert: a nextObject == c.
> self assert: c nextObject == b.
>
> I think it might help in this case, to ask for the next object before evaluating the block in the loop of #allObjectsDo:
>
>     [lastObject == object]
>         whileFalse: [
>             | nextObject |
>             nextObject := object nextObject.
>             aBlock value: object.
>             object := nextObject]
>
>
> Levente
>


I would think that materializing proxies or otherwise creating new objects within an allObjectsDo: loop is a bug, just
as it usually is whenever one grows a collection while iterating over it. It is the responsibility of the caller to
ensure that this does not happen.
Here the bug would be in #obsoleteBehaviors, which should install a copy of #isBehavior in ProtoObject before the loop
and remove it afterwards (or ProtoObject should just implement it)

Florin




Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Bert Freudenberg
On 11.01.2014, at 18:11, Florin Mateoc <[hidden email]> wrote:
> I would think that materializing proxies or otherwise creating new objects within an allObjectsDo: loop is a bug

Agreed.

> , just as it usually is whenever one grows a collection while iterating over it. It is the responsibility of the caller to
> ensure that this does not happen.
> Here the bug would be in #obsoleteBehaviors, which should install a copy of #isBehavior in ProtoObject before the loop
> and remove it afterwards (or ProtoObject should just implement it)

Or maybe it should use #isInMemory to only check the objects in memory.

- Bert -



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Bert Freudenberg



On Sat, Jan 11, 2014 at 12:05 PM, Bert Freudenberg <[hidden email]> wrote:
 
... which is when you got the 0. Or are you asking why we need the sentinel? Because if aBlock keeps allocating objects, we would never reach the end.

According to the comment in #allObjectsDo: we need the sentinel because the *execution* of aBlock will allocate an object - the activation context. So regardless of what aBlock actually does, we need the sentinel to avoid an infinite loop.

However, this bug causes the loop to skip over the sentinel. If the comment is right, we should end up in an infinite loop. But we don't, we eventually get 0. Why?

Colin 


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-cwp.660.mcz

Colin Putney-3
In reply to this post by Florin Mateoc-4



On Sat, Jan 11, 2014 at 12:11 PM, Florin Mateoc <[hidden email]> wrote:
 
I would think that materializing proxies or otherwise creating new objects within an allObjectsDo: loop is a bug, just
as it usually is whenever one grows a collection while iterating over it. It is the responsibility of the caller to
ensure that this does not happen.

If that's the case, then the caller may not send any messages to the objects enumerated. That seems overly restrictive to me. 
 
Colin
 


12