Immutability support

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

Re: Immutability support

NorbertHartl

Am 26.01.2017 um 10:32 schrieb Clément Bera <[hidden email]>:



On Thu, Jan 26, 2017 at 9:54 AM, Norbert Hartl <[hidden email]> wrote:

Am 26.01.2017 um 08:11 schrieb Clément Bera <[hidden email]>:

The "CAN'T REACH" comment is there because execution never reach that part of the code. If you write code there, it will never be executed. The process code performs a return without pushing any value on stack.

Because you return from the sender, right?

Yes because the process code returns to the sender. 

In Smalltalk a send returns necessarily a value to be pushed on the stack of the context performing the send. In the write barrier call-back, the context is not performing a send, but an instance variable store. Unlike sends, instance variable stores are not expected to push a value on stack. If the call-back returns a value, it's pushed on stack while nothing is expected, and if further code use values on stack it will use the returned value instead of the correct one, furthermore, stack depth computation is messed up, overflowing the context stack somewhere in the runtime memory leading to data corruption.

I see. I'm not sure I understand it correctly. But isn't it the case that you return the value of attemptToAssign:index: as return value of the sender? In the current implementation it will return self. This would break any method with instVar access that do not return self. 
 
Signalling an error is safe if the error is never resumed. But you'll need the returnNoValue for performance intensive modification tracking.


Ok, while you are popping of the stack there is no way to return, right? I cannot see what it has to be that way but I can see that my approach cannot work this way. In my case the error is resumed because not resuming would defeat the purpose of having transparent modification tracking.

I'll use announcements for it because it is a better fit for that anyway.

You can signal an error, but then keep the process code. For example:

Object>>#attemptToAssign: value withIndex: index 
| process |
NoModification signal.
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] forkAt: Processor activePriority + 1.
Processor yield.

this method just works fine.

However, 

Object>>#attemptToAssign: value withIndex: index 
NoModification signal.

this method does not work because it returns a value.

Ok, thanks.

Norbert


Norbert


On Wed, Jan 25, 2017 at 10:26 PM, Denis Kudriashov <[hidden email]> wrote:

2017-01-25 22:24 GMT+01:00 Denis Kudriashov <[hidden email]>:
For the Process hack, it's because the call-back was designed to return no value.

Now I am confused. 
Why anybody needs to return value from this method? 
And why there is  "CAN'T REACH" comment at the end of method?
Do you mean that method should never return? 
 
It may look like it works when returning a value but you will have uncommon crashes.

And is it safe to just signal error? 




Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

Clément Béra
In reply to this post by Clément Béra
Let's rewrite the method this way:

attemptToAssign: value withIndex: index 
        | process |
        "INSERT CODE HERE"
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] fork. 
Processor yield.
self error: 'should not be reached'.

This way it's clear that execution flow never reaches the end of the method as the error message is never sent. For some reason in the previous code by increasing the priority of the process the end of the method could be reached, as Denis showed, which leaded to incorrect behavior. This is still hackish and we still need to provide a better way of performing such returns.

Any code can be written at the  "INSERT CODE HERE" position, including exception signals, etc.

Normal methods ends with a return, for example, "^ self". When performed, "^ self" pushes "self" on the sender's stack and resumes execution in the sender. In this call-back, when reaching the end of the method, the sender's execution needs to be resumed, but no value needs to be pushed on its stack. That's why instead of a normal return I use: 
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] fork. 
Processor yield.
I don't know how to explain it better. I understand that because it was bugged it was very hard to understand. I will put a slice on the bug tracker.



On Thu, Jan 26, 2017 at 10:35 AM, Clément Bera <[hidden email]> wrote:

Sorry Clement, maybe I am stupid but it is not clear for me.
If I put halt after "CAN'T REACH" I got debugger which means that it "can reach".
Maybe by "CAN'T REACH" you mean that any return value will not be used? But it is not true: if I return something it will be result of original assignment expression. But as you said it could crash VM.

Ok I tried and I can see that. This is why I have bugs. Well "CAN'T REACH" was supposed to mean it cannot be reached, I don't understand how it could be reached.
 

On Thu, Jan 26, 2017 at 10:18 AM, Denis Kudriashov <[hidden email]> wrote:

2017-01-26 8:11 GMT+01:00 Clément Bera <[hidden email]>:
The "CAN'T REACH" comment is there because execution never reach that part of the code. If you write code there, it will never be executed. The process code performs a return without pushing any value on stack.

Sorry Clement, maybe I am stupid but it is not clear for me.
If I put halt after "CAN'T REACH" I got debugger which means that it "can reach".
Maybe by "CAN'T REACH" you mean that any return value will not be used? But it is not true: if I return something it will be result of original assignment expression. But as you said it could crash VM.
 

Signalling an error is safe if the error is never resumed. But you'll need the returnNoValue for performance intensive modification tracking.

Do you have reproducible test case to crash VM when #attemptToAssign is badly implemented? It will help for framework implementation and would be nice description for method. In comment we can point to it. Because this magic with forking process is very confusing. 
Also I not get your "returnNoValue" sentence.


Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

Stefan Marr-3
Hi Clement:

> On 26 Jan 2017, at 10:59, Clément Bera <[hidden email]> wrote:
>
> Let's rewrite the method this way:
>
> attemptToAssign: value withIndex: index
>         | process |
>         "INSERT CODE HERE"
> process := Processor activeProcess.
> [ process suspendedContext: process suspendedContext sender ] fork.
> Processor yield.
> self error: ‘should not be reached'.

The PRocessorScheduler>>#yield does not guarantee that this process becomes never runnable again.
It just puts the processes at the end of the runnable list. The fallback code of yield even explicitly forks another processes that’s signaling the old process again to make it runnable.

Best regards
Stefan



--
Stefan Marr
Johannes Kepler Universität Linz
http://stefan-marr.de/research/




Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

Denis Kudriashov
In reply to this post by Clément Béra

2017-01-26 10:59 GMT+01:00 Clément Bera <[hidden email]>:
Let's rewrite the method this way:

attemptToAssign: value withIndex: index 
        | process |
        "INSERT CODE HERE"
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] fork. 
Processor yield.
self error: 'should not be reached'.

This way it's clear that execution flow never reaches the end of the method as the error message is never sent. For some reason in the previous code by increasing the priority of the process the end of the method could be reached, as Denis showed, which leaded to incorrect behavior. This is still hackish and we still need to provide a better way of performing such returns.

Any code can be written at the  "INSERT CODE HERE" position, including exception signals, etc.

Normal methods ends with a return, for example, "^ self". When performed, "^ self" pushes "self" on the sender's stack and resumes execution in the sender. In this call-back, when reaching the end of the method, the sender's execution needs to be resumed, but no value needs to be pushed on its stack. That's why instead of a normal return I use: 
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] fork. 
Processor yield.
I don't know how to explain it better. I understand that because it was bugged it was very hard to understand. I will put a slice on the bug tracker.

Thank's Clement. Now I understand it.
Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

Denis Kudriashov
In reply to this post by Clément Béra

2017-01-26 10:59 GMT+01:00 Clément Bera <[hidden email]>:
Let's rewrite the method this way:

attemptToAssign: value withIndex: index 
        | process |
        "INSERT CODE HERE"
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] fork. 
Processor yield.
self error: 'should not be reached'.

I guess I found simplest solution to problem. Instead of process logic we can just use "thisContext sender jump".

attemptToAssign: value withIndex: index 
        | process |
  thisContext sender jump.
  self error: 'should not be reached'.

In my tests it works correctly. I use this method:

ValueHolder>>contents2: anObject
| result |
result := (contents := anObject).
self halt.
^result printString.

So debugger is opened at halt. And #result var contains anObject.
Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

Clément Béra
Denis' idea is good.

There is one problem with the implementation of jump though.

MyClass>>iv
iv := 1.
[ 2 ] value.
1halt.

MyClass new
beReadOnlyObject ;
iv

When reaching the halt we see there is an extra nil on stack.

I think this is better than the process hack, at least until we add VM support for this case.

On Thu, Jan 26, 2017 at 11:36 AM, Denis Kudriashov <[hidden email]> wrote:

2017-01-26 10:59 GMT+01:00 Clément Bera <[hidden email]>:
Let's rewrite the method this way:

attemptToAssign: value withIndex: index 
        | process |
        "INSERT CODE HERE"
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] fork. 
Processor yield.
self error: 'should not be reached'.

I guess I found simplest solution to problem. Instead of process logic we can just use "thisContext sender jump".

attemptToAssign: value withIndex: index 
        | process |
  thisContext sender jump.
  self error: 'should not be reached'.

In my tests it works correctly. I use this method:

ValueHolder>>contents2: anObject
| result |
result := (contents := anObject).
self halt.
^result printString.

So debugger is opened at halt. And #result var contains anObject.

Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

philippeback
In reply to this post by Clément Béra
Guess the method is wrong given the comment.

Shouldn't it be [param anyMask: 1] instead of [param anyMask: 2] ?

Phil

On Wed, Jan 25, 2017 at 2:14 PM, Clément Bera <[hidden email]> wrote:
I introduced the method #supportsWriteBarrier in Pharo 6.

You can backport it if you want:

VirtualMachine>>#supportsWriteBarrier
"Answer whether the VM observes the per-object read-only flag and consequently
aborts writes to inst vars of, and fails primitives that attempt to write, to read-only objects."

^(self parameterAt: 65)
ifNil: [false]
ifNotNil:
[:param| "In older VMs this is a boolean reflecting MULTIPLE_BYTECODE_SETS"
param isInteger "In newer VMs it is a set of integer flags, bit 1 of which is IMMUTABILITY"
ifTrue: [param anyMask: 2]
ifFalse: [false]]



On Wed, Jan 25, 2017 at 2:06 PM, [hidden email] <[hidden email]> wrote:
The "latest" Windows VM I do use has no such method.

Virtual Machine
---------------
C:\Users\Philippe\Dropbox\Sibelga\JiraAutomation\Pharo5.0\latestvm\pharo.exe
CoInterpreter * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017
StackToRegisterMappingCogit * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017
VM: 201701151442 https://github.com/pharo-project/pharo-vm.git $ Date: Sun Jan 15 15:42:39 2017 +0100 $ Plugins: 201701151442 https://github.com/pharo-project/pharo-vm.git $

Win32 built on Jan 15 2017 15:59:52 CUT Compiler: 5.4.0
VMMaker versionString VM: 201701151442 https://github.com/pharo-project/pharo-vm.git $ Date: Sun Jan 15 15:42:39 2017 +0100 $ Plugins: 201701151442 https://github.com/pharo-project/pharo-vm.git $
CoInterpreter * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017
StackToRegisterMappingCogit * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017

Inline image 1


On Wed, Jan 25, 2017 at 1:54 PM, Clément Bera <[hidden email]> wrote:


On Wed, Jan 25, 2017 at 11:35 AM, Norbert Hartl <[hidden email]> wrote:
Does anyone know the state of immutability support in vm and image? The latest vm downloadable is compiled with

IMMUTABILITY=1

(Esteban said that). When I open a pharo6 image with this VM and do:

ASUser new
        setIsReadOnlyObject: true;
        name: 'foo'

with

ASUser>>#name: arg1
        name := arg1

I don't get an exception. Is there something missing or am I not understanding?

Hi Norbert,

Thank you very much for looking read-only objects.

When mutating an instance variable, the VM triggers a call-back that by default does nothing. In your case, running your code does not raise an exception but the object should not be modified either. If you want an exception, you need to change the call-back code, i.e., the method Object>>#attemptToAssign: value withIndex: index. For example, you could write:

Object>>#attemptToAssign: value withIndex: index 
| process |
self notify: 'object changed !'.
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] forkAt: Processor activePriority + 1.
Processor yield.

Then, your code should open a notification window with 'object changed', and proceeding keeps running the code without mutating the object.

One needs to build a ModificationTracker framework on top of the VM support I introduced. Multiple things are required, like default behavior in this call-back and in primitive failure code. I am willing to support and help anyone willing to build such a framework, but I won't build it myself.

If you have any other questions or if you find bug don't hesitate to ask further questions

Best,

PS: Make sure "Smalltalk vm supportsWriteBarrier" answers true in your system, if this is not the case it means the VM does not support read-only objects.

Clement




 

Norbert




Reply | Threaded
Open this post in threaded view
|

Re: Immutability support

Denis Kudriashov
Now we have integrated case 19613 with VisualWorks names. 
But I propose better names 19614. It would be nice if you put some notes about it (at issue page).

2017-01-26 22:39 GMT+01:00 [hidden email] <[hidden email]>:
Guess the method is wrong given the comment.

Shouldn't it be [param anyMask: 1] instead of [param anyMask: 2] ?

Phil

On Wed, Jan 25, 2017 at 2:14 PM, Clément Bera <[hidden email]> wrote:
I introduced the method #supportsWriteBarrier in Pharo 6.

You can backport it if you want:

VirtualMachine>>#supportsWriteBarrier
"Answer whether the VM observes the per-object read-only flag and consequently
aborts writes to inst vars of, and fails primitives that attempt to write, to read-only objects."

^(self parameterAt: 65)
ifNil: [false]
ifNotNil:
[:param| "In older VMs this is a boolean reflecting MULTIPLE_BYTECODE_SETS"
param isInteger "In newer VMs it is a set of integer flags, bit 1 of which is IMMUTABILITY"
ifTrue: [param anyMask: 2]
ifFalse: [false]]



On Wed, Jan 25, 2017 at 2:06 PM, [hidden email] <[hidden email]> wrote:
The "latest" Windows VM I do use has no such method.

Virtual Machine
---------------
C:\Users\Philippe\Dropbox\Sibelga\JiraAutomation\Pharo5.0\latestvm\pharo.exe
CoInterpreter * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017
StackToRegisterMappingCogit * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017
VM: 201701151442 https://github.com/pharo-project/pharo-vm.git $ Date: Sun Jan 15 15:42:39 2017 +0100 $ Plugins: 201701151442 https://github.com/pharo-project/pharo-vm.git $

Win32 built on Jan 15 2017 15:59:52 CUT Compiler: 5.4.0
VMMaker versionString VM: 201701151442 https://github.com/pharo-project/pharo-vm.git $ Date: Sun Jan 15 15:42:39 2017 +0100 $ Plugins: 201701151442 https://github.com/pharo-project/pharo-vm.git $
CoInterpreter * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017
StackToRegisterMappingCogit * VMMaker.oscog-eem.2090 uuid: 63a161b9-17e1-4911-a89a-1687d9ba9a1a Jan 15 2017

Inline image 1


On Wed, Jan 25, 2017 at 1:54 PM, Clément Bera <[hidden email]> wrote:


On Wed, Jan 25, 2017 at 11:35 AM, Norbert Hartl <[hidden email]> wrote:
Does anyone know the state of immutability support in vm and image? The latest vm downloadable is compiled with

IMMUTABILITY=1

(Esteban said that). When I open a pharo6 image with this VM and do:

ASUser new
        setIsReadOnlyObject: true;
        name: 'foo'

with

ASUser>>#name: arg1
        name := arg1

I don't get an exception. Is there something missing or am I not understanding?

Hi Norbert,

Thank you very much for looking read-only objects.

When mutating an instance variable, the VM triggers a call-back that by default does nothing. In your case, running your code does not raise an exception but the object should not be modified either. If you want an exception, you need to change the call-back code, i.e., the method Object>>#attemptToAssign: value withIndex: index. For example, you could write:

Object>>#attemptToAssign: value withIndex: index 
| process |
self notify: 'object changed !'.
process := Processor activeProcess.
[ process suspendedContext: process suspendedContext sender ] forkAt: Processor activePriority + 1.
Processor yield.

Then, your code should open a notification window with 'object changed', and proceeding keeps running the code without mutating the object.

One needs to build a ModificationTracker framework on top of the VM support I introduced. Multiple things are required, like default behavior in this call-back and in primitive failure code. I am willing to support and help anyone willing to build such a framework, but I won't build it myself.

If you have any other questions or if you find bug don't hesitate to ask further questions

Best,

PS: Make sure "Smalltalk vm supportsWriteBarrier" answers true in your system, if this is not the case it means the VM does not support read-only objects.

Clement




 

Norbert





123