Smalltalk › Squeak › Squeak - Dev

The Trunk: System-dtl.685.mcz

‹ Previous Topic Next Topic ›
 
classic Classic list List threaded Threaded
♦
♦
 8 messages Options
commits-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

The Trunk: System-dtl.685.mcz

commits-2
David T. Lewis uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-dtl.685.mcz

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

Name: System-dtl.685
Author: dtl
Time: 7 September 2014, 4:58:15.377 pm
UUID: 55eba121-23d3-41cc-9525-52554adac2ec
Ancestors: System-eem.684

Smalltalk isRunningCog should answer false for an interpreter VM

=============== Diff against System-eem.684 ===============

Item was changed:
  ----- Method: SmalltalkImage>>isRunningCog (in category 'system attributes') -----
  isRunningCog
  "Answers if we're running on a Cog VM (JIT or StackInterpreter)"
 
+ ^( [self vmParameterAt: 42] on: Error do: [] )
- ^(self vmParameterAt: 42)
  ifNil: [false]
  ifNotNil: [:numStackPages| numStackPages > 0]!


Eliot Miranda-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

Eliot Miranda-2


On Sun, Sep 7, 2014 at 1:58 PM, <[hidden email]> wrote:
David T. Lewis uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-dtl.685.mcz

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

Name: System-dtl.685
Author: dtl
Time: 7 September 2014, 4:58:15.377 pm
UUID: 55eba121-23d3-41cc-9525-52554adac2ec
Ancestors: System-eem.684

Smalltalk isRunningCog should answer false for an interpreter VM

=============== Diff against System-eem.684 ===============

Item was changed:
  ----- Method: SmalltalkImage>>isRunningCog (in category 'system attributes') -----
  isRunningCog
        "Answers if we're running on a Cog VM (JIT or StackInterpreter)"

+       ^( [self vmParameterAt: 42] on: Error do: [] )
-       ^(self vmParameterAt: 42)
                ifNil: [false]
                ifNotNil: [:numStackPages| numStackPages > 0]!

Instead of creating an exception handler why not modify the interpreter to answer nil?  I know the answer is that the Interpreter doesn't, and fails.  But we've had years in which we could change the Interpreter to match vmParameterAt: and instead of doing the good thing (have the Interpreter answer nil for values answered by Cog VMs), we force the standard system to waste time.  We should optimize the common case (trunk running on Cog) not penalize it to cover the uncommon case (trunk running on Interpreter VM). 
--
best,
Eliot
Bert Freudenberg
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

Bert Freudenberg
On 08.09.2014, at 20:45, Eliot Miranda <[hidden email]> wrote:

On Sun, Sep 7, 2014 at 1:58 PM, <[hidden email]> wrote:
David T. Lewis uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-dtl.685.mcz

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

Name: System-dtl.685
Author: dtl
Time: 7 September 2014, 4:58:15.377 pm
UUID: 55eba121-23d3-41cc-9525-52554adac2ec
Ancestors: System-eem.684

Smalltalk isRunningCog should answer false for an interpreter VM

=============== Diff against System-eem.684 ===============

Item was changed:
  ----- Method: SmalltalkImage>>isRunningCog (in category 'system attributes') -----
  isRunningCog
        "Answers if we're running on a Cog VM (JIT or StackInterpreter)"

+       ^( [self vmParameterAt: 42] on: Error do: [] )
-       ^(self vmParameterAt: 42)
                ifNil: [false]
                ifNotNil: [:numStackPages| numStackPages > 0]!

Instead of creating an exception handler why not modify the interpreter to answer nil?  I know the answer is that the Interpreter doesn't, and fails.  But we've had years in which we could change the Interpreter to match vmParameterAt: and instead of doing the good thing (have the Interpreter answer nil for values answered by Cog VMs), we force the standard system to waste time.  We should optimize the common case (trunk running on Cog) not penalize it to cover the uncommon case (trunk running on Interpreter VM). 

Nah, if performance really was an issue we could just remove the primitiveFailed in vmParameterAt:. No need to change the VM.

- Bert -






smime.p7s (5K) Download Attachment
Chris Muller-3
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

Chris Muller-3
I'm not one to tread into you VM guys' domain but I just wanted to
express my agreement with Eliot.  "Performance" isn't the only viable
criticism of using on: Error do: [ ... ].  It's just plain bad style,
bad code.  We know better than to do that.


On Mon, Sep 8, 2014 at 1:58 PM, Bert Freudenberg <[hidden email]> wrote:

> On 08.09.2014, at 20:45, Eliot Miranda <[hidden email]> wrote:
>
> On Sun, Sep 7, 2014 at 1:58 PM, <[hidden email]> wrote:
>>
>> David T. Lewis uploaded a new version of System to project The Trunk:
>> http://source.squeak.org/trunk/System-dtl.685.mcz
>>
>> ==================== Summary ====================
>>
>> Name: System-dtl.685
>> Author: dtl
>> Time: 7 September 2014, 4:58:15.377 pm
>> UUID: 55eba121-23d3-41cc-9525-52554adac2ec
>> Ancestors: System-eem.684
>>
>> Smalltalk isRunningCog should answer false for an interpreter VM
>>
>> =============== Diff against System-eem.684 ===============
>>
>> Item was changed:
>>   ----- Method: SmalltalkImage>>isRunningCog (in category 'system
>> attributes') -----
>>   isRunningCog
>>         "Answers if we're running on a Cog VM (JIT or StackInterpreter)"
>>
>> +       ^( [self vmParameterAt: 42] on: Error do: [] )
>> -       ^(self vmParameterAt: 42)
>>                 ifNil: [false]
>>                 ifNotNil: [:numStackPages| numStackPages > 0]!
>
>
> Instead of creating an exception handler why not modify the interpreter to
> answer nil?  I know the answer is that the Interpreter doesn't, and fails.
> But we've had years in which we could change the Interpreter to match
> vmParameterAt: and instead of doing the good thing (have the Interpreter
> answer nil for values answered by Cog VMs), we force the standard system to
> waste time.  We should optimize the common case (trunk running on Cog) not
> penalize it to cover the uncommon case (trunk running on Interpreter VM).
>
>
> Nah, if performance really was an issue we could just remove the
> primitiveFailed in vmParameterAt:. No need to change the VM.
>
> - Bert -
>
>
>
>
>
>

David T. Lewis
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

David T. Lewis
A method that tests for "is running Cog" and that blows up if you are not
running Cog is not a very sensible thing. If you can make it faster and
prettier that's great, but please do check to make sure it actually works.

Dave

> I'm not one to tread into you VM guys' domain but I just wanted to
> express my agreement with Eliot.  "Performance" isn't the only viable
> criticism of using on: Error do: [ ... ].  It's just plain bad style,
> bad code.  We know better than to do that.
>
>
> On Mon, Sep 8, 2014 at 1:58 PM, Bert Freudenberg <[hidden email]>
> wrote:
>> On 08.09.2014, at 20:45, Eliot Miranda <[hidden email]> wrote:
>>
>> On Sun, Sep 7, 2014 at 1:58 PM, <[hidden email]> wrote:
>>>
>>> David T. Lewis uploaded a new version of System to project The Trunk:
>>> http://source.squeak.org/trunk/System-dtl.685.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: System-dtl.685
>>> Author: dtl
>>> Time: 7 September 2014, 4:58:15.377 pm
>>> UUID: 55eba121-23d3-41cc-9525-52554adac2ec
>>> Ancestors: System-eem.684
>>>
>>> Smalltalk isRunningCog should answer false for an interpreter VM
>>>
>>> =============== Diff against System-eem.684 ===============
>>>
>>> Item was changed:
>>>   ----- Method: SmalltalkImage>>isRunningCog (in category 'system
>>> attributes') -----
>>>   isRunningCog
>>>         "Answers if we're running on a Cog VM (JIT or
>>> StackInterpreter)"
>>>
>>> +       ^( [self vmParameterAt: 42] on: Error do: [] )
>>> -       ^(self vmParameterAt: 42)
>>>                 ifNil: [false]
>>>                 ifNotNil: [:numStackPages| numStackPages > 0]!
>>
>>
>> Instead of creating an exception handler why not modify the interpreter
>> to
>> answer nil?  I know the answer is that the Interpreter doesn't, and
>> fails.
>> But we've had years in which we could change the Interpreter to match
>> vmParameterAt: and instead of doing the good thing (have the Interpreter
>> answer nil for values answered by Cog VMs), we force the standard system
>> to
>> waste time.  We should optimize the common case (trunk running on Cog)
>> not
>> penalize it to cover the uncommon case (trunk running on Interpreter
>> VM).
>>
>>
>> Nah, if performance really was an issue we could just remove the
>> primitiveFailed in vmParameterAt:. No need to change the VM.
>>
>> - Bert -
>>
>>
>>
>>
>>
>>
>



Eliot Miranda-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

Eliot Miranda-2
In reply to this post by Bert Freudenberg


On Mon, Sep 8, 2014 at 11:58 AM, Bert Freudenberg <[hidden email]> wrote:
On 08.09.2014, at 20:45, Eliot Miranda <[hidden email]> wrote:

On Sun, Sep 7, 2014 at 1:58 PM, <[hidden email]> wrote:
David T. Lewis uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-dtl.685.mcz

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

Name: System-dtl.685
Author: dtl
Time: 7 September 2014, 4:58:15.377 pm
UUID: 55eba121-23d3-41cc-9525-52554adac2ec
Ancestors: System-eem.684

Smalltalk isRunningCog should answer false for an interpreter VM

=============== Diff against System-eem.684 ===============

Item was changed:
  ----- Method: SmalltalkImage>>isRunningCog (in category 'system attributes') -----
  isRunningCog
        "Answers if we're running on a Cog VM (JIT or StackInterpreter)"

+       ^( [self vmParameterAt: 42] on: Error do: [] )
-       ^(self vmParameterAt: 42)
                ifNil: [false]
                ifNotNil: [:numStackPages| numStackPages > 0]!

Instead of creating an exception handler why not modify the interpreter to answer nil?  I know the answer is that the Interpreter doesn't, and fails.  But we've had years in which we could change the Interpreter to match vmParameterAt: and instead of doing the good thing (have the Interpreter answer nil for values answered by Cog VMs), we force the standard system to waste time.  We should optimize the common case (trunk running on Cog) not penalize it to cover the uncommon case (trunk running on Interpreter VM). 

Nah, if performance really was an issue we could just remove the primitiveFailed in vmParameterAt:. No need to change the VM.

That's a good idea.  Change the primitiveFailed to ^nil.

--
best,
Eliot
David T. Lewis
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

David T. Lewis
On Mon, Sep 08, 2014 at 01:09:17PM -0700, Eliot Miranda wrote:

> On Mon, Sep 8, 2014 at 11:58 AM, Bert Freudenberg <[hidden email]>
> wrote:
>
> > On 08.09.2014, at 20:45, Eliot Miranda <[hidden email]> wrote:
> >
> > Instead of creating an exception handler why not modify the interpreter to
> > answer nil?  I know the answer is that the Interpreter doesn't, and fails.
> >  But we've had years in which we could change the Interpreter to match
> > vmParameterAt: and instead of doing the good thing (have the Interpreter
> > answer nil for values answered by Cog VMs), we force the standard system to
> > waste time.  We should optimize the common case (trunk running on Cog) not
> > penalize it to cover the uncommon case (trunk running on Interpreter VM).
> >

All of the VMs, including Cog, behave identically in this regard. If you
request a VM parameter that is not understood by the VM, the primitive fails.

> >
> > Nah, if performance really was an issue we could just remove the
> > primitiveFailed in vmParameterAt:. No need to change the VM.
> >
>
> That's a good idea.  Change the primitiveFailed to ^nil.
>

It would be a good idea if performance was actually an issue. It is not.

This is a case where using exceptions makes good sense. If some arbitrary
VM cannot satisfy a primitive request, it should fail the primitive.
Answering nil as a flag for primitive failed is bad karma, and should
not be done unless there is a good reason for doing so. Improving the
performance of #isRunningCog is not a good reason.

BTW there is another method with the same problem:

  isRunningCogit
    "Answers if we're running on the Cog JIT"

There are no senders of #isRunningCogit in the image, so it does not need
to be fixed.  But the #isRunningCog bug was breaking the unit tests, so
it needed fixing.

Dave


Bert Freudenberg
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: The Trunk: System-dtl.685.mcz

Bert Freudenberg

On 09.09.2014, at 04:49, David T. Lewis <[hidden email]> wrote:

> On Mon, Sep 08, 2014 at 01:09:17PM -0700, Eliot Miranda wrote:
>> On Mon, Sep 8, 2014 at 11:58 AM, Bert Freudenberg <[hidden email]>
>> wrote:
>>
>>> On 08.09.2014, at 20:45, Eliot Miranda <[hidden email]> wrote:
>>>
>>> Instead of creating an exception handler why not modify the interpreter to
>>> answer nil?  I know the answer is that the Interpreter doesn't, and fails.
>>> But we've had years in which we could change the Interpreter to match
>>> vmParameterAt: and instead of doing the good thing (have the Interpreter
>>> answer nil for values answered by Cog VMs), we force the standard system to
>>> waste time.  We should optimize the common case (trunk running on Cog) not
>>> penalize it to cover the uncommon case (trunk running on Interpreter VM).
>>>
>
> All of the VMs, including Cog, behave identically in this regard. If you
> request a VM parameter that is not understood by the VM, the primitive fails.
>
>>>
>>> Nah, if performance really was an issue we could just remove the
>>> primitiveFailed in vmParameterAt:. No need to change the VM.
>>>
>>
>> That's a good idea.  Change the primitiveFailed to ^nil.
>>
>
> It would be a good idea if performance was actually an issue. It is not.
>
> This is a case where using exceptions makes good sense. If some arbitrary
> VM cannot satisfy a primitive request, it should fail the primitive.
> Answering nil as a flag for primitive failed is bad karma, and should
> not be done unless there is a good reason for doing so. Improving the
> performance of #isRunningCog is not a good reason.
>
> BTW there is another method with the same problem:
>
>  isRunningCogit
>    "Answers if we're running on the Cog JIT"
>
> There are no senders of #isRunningCogit in the image, so it does not need
> to be fixed.  But the #isRunningCog bug was breaking the unit tests, so
> it needed fixing.
>
> Dave
+1

- Bert -






smime.p7s (5K) Download Attachment
Free forum by Nabble Edit this page