[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Help-smalltalk] [PATCH] Process
From: |
Holger Hans Peter Freyther |
Subject: |
Re: [Help-smalltalk] [PATCH] Process |
Date: |
Mon, 24 Mar 2014 10:50:38 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Mar 24, 2014 at 09:22:19AM +0100, Gwenaël Casaccio wrote:
> + Termination isNil ifFalse: [ ^ Termination ].
> + ^ [
> + Termination isNil ifTrue: [ Termination := MethodContext stack:
> 4 flags: 6 method: UndefinedObject>>#__terminate ip: 0 sp: -1 ].
> + Termination
> + ] valueWithoutPreemption
> + ]
^Termination ifNil: [
Termination := ..
].
?
> - makeUntrusted: aBoolean [
> - "Set whether the receiver is trusted or not."
> -
> - <category: 'basic'>
> - | ctx |
> - ctx := self context.
> - [ctx isNil] whileFalse:
> - [ctx makeUntrusted: aBoolean.
> - ctx := ctx parentContext]
> - ]
let's remove this separately and rhight now? Care to send a patch?
> - self suspend
> + self suspend.
unrelated patch. Please use git add -p and aim for minimal patches.
> - | activePriority |
> - activePriority := Processor activePriority.
> + | oldPriority |
> + oldPriority := priority.
tabs vs. spaces? Separate patch/fix as well.
> priority := anInteger.
> "Atomically move the process to the right list, preempting
> the current process if necessary."
> - self isReady ifTrue: [self resume].
> + (myList == (Processor processesAt: oldPriority)) ifTrue: [self
> primResume: false].
Okay. So this has always been broken. Do you want to create a separate
patch with the old self resume for it?
> - [aBlock on: ProcessBeingTerminated
> + [aBlock on: SystemExceptions.ProcessBeingTerminated
Why is that?
> - startExecution: aDirectedMessage [
Is this code dead? Can we remove it right now?
> onBlock: aBlockClosure at: aPriority suspend: aBoolean [
> <category: 'private'>
> - "It is important to retrieve this before we start the
> - process, because we want to choose whether to continue
> - running the new process based on the *old* activePriority,
> - not the one of the new process which is the maximum one."
> -
> - | closure activePriority |
> - activePriority := Processor activePriority.
> - closure :=
> - [[[
> - "#priority: is inlined for two reasons. First, to be able to
> - suspend the process, and second because we need to invert
> - the test on activePriority! This because here we may want to
> - yield to the creator, while in #priority: we may want to yield
> - to the process whose priority was changed."
> - priority := aPriority.
> - aBoolean
> - ifTrue: [self suspend]
> - ifFalse: [
> - aPriority < activePriority ifTrue: [ Processor yield ]
> ].
> - aBlockClosure value]
> - on: SystemExceptions.ProcessBeingTerminated
> - do:
> - [:sig |
> - "If we terminate in the handler, the 'ensure'
> blocks are not
> - evaluated. Instead, if the handler returns, the
> unwinding
> - is done properly."
> -
> - sig return]]
> - ensure: [self primTerminate]].
> -
> - "Start the Process immediately so that we get into the
> - #on:do: handler. Otherwise, we will not be able to
> - terminate the process with #terminate. The #resume will
> - preempt the forking process."
> - suspendedContext := closure asContext: nil.
> - priority := Processor unpreemptedPriority.
> - self
> - addToBeFinalized;
> - resume
> +
> + | closure |
> + closure := [ [ aBlockClosure value ] ensure: [ self primTerminate ] ].
> + suspendedContext := closure asContext: (self class termination) copy.
> + priority := aPriority.
> + self addToBeFinalized.
> + aBoolean ifTrue: [ ^ self ].
> + self primResume: false
tabs vs. spaces. And are you sure we can really simplify it that much? I need to
have a closer look ath this essential part.
> +TestCase subclass: TestProcess [
> + self assert: p_1 isSuspended.
> + 1 to: 9 do: [ :i | self assert: ((Processor processesAt: i)
> includes: p_1) not ].
Can we have Process return the interval of valid priorities.
> +
> + p_1 := [
> + ] fork.
> +
> + self assert: p_1 isTerminated.
hmm. I don't think test will be quite flaky in terms of scheduling and other
parts. Do you think we can mock/test this in a more reliable way?
> +
> + ok := #test_creation.
> + p_1 := [
> + ok := #p_13.
> + ] forkAt: 2.
> +
> + self assert: ok = #test_creation.
same here and probably the other tests.