Take Two - Primitive retry across suspension for OwnedLock waitAcquire - Vm-dev

11 Jun 2016

      I've thrown away previous attempts including:
* pushing primFailCode via the stack;
* adding new instance variables to Process to store primFail,
newMethod, primitiveFunctionPointer across context switches.
* naively decrementing the instructionPointer
and found a promising stepping-stone to alternative solution, which
I'm hoping to get some feedback on.
Using the following sample code (per attached changeset)...
...
OwnedLock>>experiment1
    | result |
    result := OrderedCollection new: 20.
    result myAdd: 0.
[result myAdd: 11.
    [result myAdd: 12.
     self experimentSuccessAndSleep] on: Error do: [result myAdd: 13].
result myAdd: 14] forkAt: 72.

[result myAdd: 21.
    [result myAdd: 22.
     self experimentFailAndWakeOther] on: Error do: [result myAdd: 23].
result myAdd: 24] forkAt: 71.

result myAdd: 8.
self experimentSuccessAndWakeOther.
result myAdd: 9.
^result.

...
Where the primitives are...
primitiveExperimentSuccessAndSleep
    | ownedLock activeProc |
    ownedLock := self stackTop.
    activeProc := self activeProcess.
    self addLastLink: activeProc toList: ownedLock.
    self transferTo: self wakeHighestPriority.
primitiveExperimentFailAndWakeOther
    | ownedLock waitingProcess |
    ownedLock := self stackTop.
    self primitiveFailFor: 42.
    (self isEmptyList: ownedLock)
        ifFalse:
            [waitingProcess := self removeFirstLinkOfList: ownedLock.
            self resume: waitingProcess preemptedYieldingIf: preemptionYields]
primitiveExperimentSuccessAndWakeOther
    | ownedLock waitingProcess |
    ownedLock := self stackTop.  "rcvr"
    (self isEmptyList: ownedLock)
       ifFalse:
        [waitingProcess := self removeFirstLinkOfList: ownedLock.
        self resume: waitingProcess preemptedYieldingIf: preemptionYields]
and inputting the following to the "reader.image" ...
    o := OwnedLockExperiment new.
    o experiment1.   !
I find that unaltered StackVM and CogVM differ in their results.
The StackVM produces...
    an OrderedCollection(0 11 12 21 22 13 14 24 8 9)
while CogVM produces...
    an OrderedCollection(0 11 12 21 22 14 24 8 9)
The StackVM can be aligned with the CogVM by appending "self
initPrimCall" to the end of StackInterpreter>>transferTo:
after which the StackVM produces...
    an OrderedCollection(0 11 12 21 22 14 24 8 9)
However I propose and request for comment(!) that the following result
is preferable...
    OrderedCollection(0 11 12 21 22 14 23 24 8 9)
and I've found a way to do it.
----------------------------------------------------
Observing the context switch execution flow for the StackVM (with a
halt in primitiveExperimentFailAndWakeOther)...
   commonSendOrdinary
     internalFindNewMethodOrdinary
     internalExecuteNewMethod
     .  slowPrimitiveResponse
     .  .   dispatchFunctionPointer:
     .  .      primitiveExperimentFailAndWakeOther
     .  .         primitiveFailFor:
     .  .         resume:preemptedYieldingIf:
     .  .            putToSleep:yieldingIf
     .  .            transferTo:
                          CONTEXT SWITCH
     .  .                "initPrimCall added to align StackVM with CogVM"
     .  .   maybeRetryPrimitiveOnFailure
     .  succeeded ifTrue:[self browserPluginReturnIfNeeded. ^nil]].
     .  "if not primitive, or primitive failed, activate the method"
     .  self internalActivateNewMethod
     fetchNextBytecode
the difference in #experiment1 between the StackVM and CogVM was
primFailCode set by primitiveFailFor: before the context switch was
being handled by restored process.
But another way to deal with this would be to delay the context switch
until *after* the primitive failure was handled, with an execution
flow like this...
   commonSendOrdinary
     internalFindNewMethodOrdinary
     internalExecuteNewMethod
     .  slowPrimitiveResponse
     .  .   dispatchFunctionPointer:
     .  .      primitiveExperimentFailAndWakeOther
     .  .         primitiveFailFor:
     .  .         resume:preemptedYieldingIf:
     .  .            putToSleep:yieldingIf
     .  .            transferTo:
                          FLAG DELAYED CONTEXT SWITCH
     .  .                "initPrimCall added to align StackVM with CogVM"
     .  .   maybeRetryPrimitiveOnFailure
     .  succeeded ifTrue:[self browserPluginReturnIfNeeded. ^nil]].
     .  "if not primitive, or primitive failed, activate the method"
     .  self internalActivateNewMethod
     CONTEXT SWITCH
     fetchNextBytecode
The attached ExperimentPrimitives-minimal.2.cs
makes this change for the StackVM such that #experiment1 now produces...
    an OrderedCollection(0 11 12 21 22 14 23 24 8 9)
as proposed above.  In this way, the primFailCode doesn't need to be
carried across the context switch.  It is fully dealt with before the
context switch.
Now the reason I'm particularly interested in this, is that the
context switch occurs immediately before the fetchNextBytecode at the
end of the dispatch loop, and *possibly* provides the opportunity to
*not* fetchNextBytecode when a process is woken up.  For example,
re-entering primitiveOwnedLockWaitAcquire when the process wakes up,
so a lock can only be acquired by the activeProcess, and
primitiveOwnedLockRelease does not assign locks to sleeping processes,
it *only* releases the lock.
But besides that, I believe my proposed change may provide some
general benefit even if my plans for primitiveOwnedLockWaitAcquire
don't pan out.
REVIEW NOTES
1. Run buildExperimentReaderImage.sh to produce experiment-reader.image
        manually file in  Experiment.2.cs  then [save&quit]
2. Load ExperimentPrimitives-minimal.3.cs into SpurVMMaker.image
3. Run Stack and Cog simulators as follows..
| cos |
cos := StackInterpreterSimulator newWithOptions: #(ObjectMemory
Spur32BitMemoryManager).
cos desiredNumStackPages: 8.
cos openOn: 'experiment-reader.image'.
cos openAsMorph; run
| cos |
cos := CogVMSimulator newWithOptions:
#(Cogit StackToRegisterMappingCogit "SimpleStackBasedCogit"
ObjectMemory Spur32BitCoMemoryManager "ISA ARMv5" "ISA IA32").
"cos initializeThreadSupport."
cos desiredNumStackPages: 8.
cos openOn: 'experiment-reader.image'.
cos openAsMorph; run
4. Run this simulator code...
o := OwnedLockExperiment new.
o experiment1. !
Initially there are no changes. The changes are wrapped by variable
delayedTransferEnabled.
Uncomment the halt in StackInterpreter>>commonSendOrdinary to provide
the opportunity to enable the new code.
Uncomment "delayedTransferEnabled := true" in: commonSendDynamicSuper;
commonSendImplicitReceiver; commonSendOrdinary; commonSendOuter: to
test all the way from image bootup. (I don't know if those four really
need the mod, but they all similarly called #internalExecuteNewMethod,
so its a guess it is)
cheers -ben