pharo icon indicating copy to clipboard operation
pharo copied to clipboard

Unified "status" concept for Processes

Open daniels220 opened this issue 9 months ago • 10 comments

Describe the request Process has methods like isSuspended, isTerminated, isActiveProcess, but they do not discriminate between all possible states a process can be in, and it's not always clear how their truth values relate. Dolphin Smalltalk has a method Process>>status (I think—might be #state) which returns a Symbol from the set:

  • #running: the active process
  • #ready: waiting in the ProcessorScheduler priority lists and will run as soon as processor time is available
  • #waiting: waiting on a Semaphore
  • #suspended: new process not yet resumed or has been sent #suspend
  • #dead: terminated
  • #debug: debugger attached

These states are mutually exclusive and clearly enumerate all possible states a process can be in. There are corresponding isWaiting, isReady, etc. methods as well. Something like this would be very useful in Pharo.

Expected behavior For Pharo, we additionally have isTerminating, which still answers true once the process has finished terminating. We could either change this but add an isTerminatingOrTerminated, and make #terminating a possible status, or just leave it alone as an extra on the side and have #dead/#terminated status only apply to fully-terminated processes.

#debug status carries significant extra complexity so could be omitted for now, though it would be very useful in general.

Expected development cost The implementations of isWaiting and isReady, the two missing test methods, are straightforward as far as I can tell, and from there building a #status implementation isn't too hard. I would want review from someone familiar with the VM to ensure edge cases don't slip through though.

Version information: Any OS, any Pharo

daniels220 avatar Oct 06 '23 19:10 daniels220

Oh—I've been thinking of this for a while, just based on my experience with Dolphin, but what specifically prompted it was #14701/#14905—see comment there.

daniels220 avatar Oct 09 '23 16:10 daniels220

I like the idea!

MarcusDenker avatar Oct 10 '23 08:10 MarcusDenker

Yes we should revise this part.

Ducasse avatar Nov 19 '23 21:11 Ducasse

@isCzech are you interested in this issue?

Ducasse avatar Nov 19 '23 21:11 Ducasse

Hi, thanks for reminding me of this issue; I've already saved it for long winter evenings.

I agree having an exhaustive set of five mutually exclusive states a process can be in at any given moment would be helpful when learning (I struggled to internalize the concept of a process without having all five is methods (*) corresponding to the states).

(*) isRunning, isReady, isBlocked, isSuspended, isTerminated

As regards isTerminating and the #terminating status I don't think it's necessary and not very useful any longer (after fixing #terminate). I suspect it was introduced to bypass problems with termination in the past and I plan to revisit this issue and attempt to eliminate the #terminating for good :) For the moment it can be ignored in this discussion.

My take on the two missing is methods is:

isBlocked
	"Answer true if blocked on a condition variable.
	A process is blocked if it is waiting on some list
	(e.g. a Semaphore), other than the runnable process lists."

	myList ifNotNil: [ :list | ^list class ~~ ProcessList ].
	^false
isReady
	self isRunning ifTrue: [ ^false ].
	self isTerminated ifTrue: [ ^false ].
	^myList class == ProcessList

I prefer isBlocked because isReady (aka runnable) processes are also "waiting" on a list :) Also, isBlocked should not restrict to Semaphores only, they could wait on any condition variable like a Mutex etc.

I haven't felt a need for a #status method yet but I'll check Dolphin's use.

Thanks for this issue, Squeak and Cuis have all five states covered (with a little inconsistency in Squeak where isRunnable corresponds to isReady PLUS isRunning).

Best, Jaromir

isCzech avatar Nov 19 '23 23:11 isCzech

@isCzech, and good to hear #isTerminating can be ignored, it did seem like a workaround that would be better not to expose to the "user"—if you can eliminate it altogether, even better. Those two implementations looks like what I came up with, though you're right that testing against ProcessList should be more robust than specifying Semaphore (with opposite truthiness ofc)—good thinking!

One thing that I think is very important is to be extremely sure that the five statuses really are mutually exclusive. Dolphin ensures this this by making the #state method the single source of truth, with the others being e.g.

isWaiting
	"Answer whether the receiver is waiting on a Semaphore."

	^self state == #waiting

This also reduces code duplication—your isBlocked and isReady both test myList class against ProcessList, which could be a single branch in a combined method. And in fact, looking at the implementation of isSuspended, it already checks isActiveProcess and isTerminated before doing its own work—so effectively, there's a natural order of the tests required to determine process state, and each testing method performs all of the ones up to that point in the order. It seems like much better self-documentation in a lot of ways to lay out the entire decision tree in one method, which I believe would be:

Process>>state

	self isActiveProcess ifTrue: [ ^ #active ].
	(suspendedContext isNil or: [
		 suspendedContext isDead or: [
			 suspendedContext method == (self class >> #endProcess) ] ])
		ifTrue: [ ^ #terminated ].
	(myList isNil or: [ myList isEmpty ]) ifTrue: [ ^ #suspended ].
	^ myList class == ProcessList
		  ifTrue: [ #ready ]
		  ifFalse: [ #blocked ]

daniels220 avatar Nov 20 '23 18:11 daniels220

Hi @daniels220

[...] very important is to be extremely sure that the five statuses really are mutually exclusive. Dolphin ensures this this by making the #state method the single source of truth

This is a great point; I spent quite a lot of time verifying the states are really mutually exclusive :)

This also reduces code duplication [...]

In case of #terminated and #suspended yes, but in case of #ready and #blocked it adds unnecessary checks into the execution: #isBlocked only checks myList and is done but in your #state method it goes via checking isRunning and isTerminated. On top of that if you implement isBlocked as ^self state == #blocked it even adds a send. But maybe this doesn't really matter and readability is the winner :)

Actually, in case of isActiveProcess you prefer the fast original implementation and use isActiveProcess in the #state method and not the other way around. Is this because of the speed and fewer suspension points in ^Processor activeProcess == self?

I like the idea of Process>>#state especially because we have the mutual exclusivity - definitely a great tool when debugging processes.

Thanks, Jaromir

isCzech avatar Nov 21 '23 19:11 isCzech

#isBlocked only checks myList and is done but in your #state method it goes via checking isRunning and isTerminated.

Well, that's exactly the question—might those additional checks be necessary? It does make sense that a process that looks like it's blocked (because it thinks it's waiting on e.g. a Semaphore) shouldn't be actually dead—but can we prove that this is impossible? IOW can we prove that there will never be a process with non-nil myList but nil or dead suspendedContext? Having it all in one method at least guarantees that you will never see two isWhatever methods answer true. (The single method is also a good opportunity for a debug assertion, if the above scenario should never occur but there's concern of a VM bug.) Probably not a serious concern, but there's some pretty esoteric code around process termination...

But maybe this doesn't really matter and readability is the winner :)

That's my sense—examining the state of a process so often that this notably slows down overall execution seems like code smell to me, much better to synchronize execution using Semaphores (and higher-level structures like queues etc).

Actually, in case of isActiveProcess you prefer the fast original implementation and use isActiveProcess in the #state method and not the other way around. Is this because of the speed and fewer suspension points in ^Processor activeProcess == self?

Nah, this is just laziness in a first draft. Or rather, the reasons you give might have gone through my mind, but they're definitely premature optimization—sometimes said to be the root of all evil :). I guess the one other "reason" is just that it's the first condition—I introduce no duplication or possibility of incorrectness by writing it this way, where I would for any subsequent condition.

daniels220 avatar Nov 22 '23 03:11 daniels220

can we prove that there will never be a process with non-nil myList but nil or dead suspendedContext? Having it all in one method at least guarantees that you will never see two isWhatever methods answer true.

good point; under "normal" conditions (no mischievous tricks) a process should be in one and only one of the five states.

However, here's an example: ([Semaphore new wait] forkAt: 50) suspendedContext: nil In Squeak I have taken care of this via:

Process>>suspendedContext: aContextOrNil
	priority ifNil: [priority := Processor activePriority].
	suspendedContext := aContextOrNil ifNil: [self suspend. nil]

but in Pharo you end up with a process that is #blocked and #terminated at the same time. I haven't tried to reproduce this example in Dolphin.

If we couldn't prove or guarantee all such situations can be avoided we might have to prioritize what state has "higher priority" - #blocked or #terminated?? You chose #terminated in your draft but someone may miss that the process also occupies a Semaphore and what if that matters? (Just asking, I'm not sure it's a good example)

I wanted to reply #state is a clean and consistent tool to track process' state so let's do it but I'm no longer so sure. Maybe, in edge scenarios, e.g. when debugging, you may want to see all states of the process which would give you a hint something is wrong :)

Is this because of the speed and fewer suspension points in ^Processor activeProcess == self?

Nah, this is just laziness in a first draft

Good, then it really can be clean and consistent :) In Dolphin (v7, not sure about v8), you also have isActive inconsistent so I thought is was intentional.

examining the state of a process so often that this notably slows down overall execution seems like code smell to me

Yes, right, so this is not a concern.

isCzech avatar Nov 22 '23 17:11 isCzech

I've been busy with other things and letting this sit in the back of my mind, but at this point I'd like to close it out if we can. Regarding the potential for inconsistent states, I think (a) that's the sort of thing that anyone other than a VM developer shouldn't have to worry about, but also (b) since none of the existing tools display a list of the results of each of the #isX methods, for someone to make use of non-mutually-exclusive tests to notice a bug, they would have to already suspect that as a possibility, at which point they could easily insert a specific test for the inconsistency directly into #state itself. Honestly I think reading the implementation of state would be more likely to clue them in to the possible inconsistency than happening to notice directly that two different test methods both answer true.

So, I went ahead and made a PR for the implementation as I have it.

daniels220 avatar Jan 29 '24 23:01 daniels220