Fix #4352: javalib UnixProcessGen2 is now atomic #4353
Conversation
|
This PR is passing CI and ready for review, as time permits. A private build of |
| final val EXIT_VALUE_UNKNOWN = jl.Integer.MIN_VALUE | ||
|
|
||
| // Non-negative values indicate exit status is known to be that value. | ||
| val cached = new juc.atomic.AtomicInteger(EXIT_VALUE_UNKNOWN) |
There was a problem hiding this comment.
CachedExitValue can extends the AtomicInteger to save some allocation
There was a problem hiding this comment.
Thank you for the suggestion.
I wanted to centralize all the handling of the 'cached' value in one
place so I created a class.
One could indeed save an instance allocation by having private methods
and a local variable. Early iterations of this PR did exactly that. It reminded
me of very early Fortran II code. I was finding that having access to the
variable spread all over the place made it very difficult to tell who as
modifying the variable and when. Having methods makes the handling
much more consistent. It is harder to have a slightly wrong synchronization
is one (or more) places and miss it.
There was a problem hiding this comment.
On reading the suggestion several more times, I think I see the essence of it.
// not compiled
private class CachedExitValue() extends AtomicInteger(){
final val EXIT_VALUE_UNKNOWN = jl.Integer.MIN_VALUE
// Non-negative values indicate exit status is known to be that value.
this.set(EXIT_VALUE_UNKNOWN)
That would preserve the class and indeed reduce an allocation at Process creation.
He-Pin is that code somewhat like you suggested?
There was a problem hiding this comment.
Yes, in Java we will use Atomic*FieldUpdator to save some allocation too, I'm not sure about how Scala native encoding here, but that should not have much difference.
Fix #4352
javalib
java.lang.processUnixProcessGen2now usesjava.util.concurrent.atomicmethodsto guard a possibly shared exit status.
Previously, it
synchronizedon theProcessinstance. The current approach should belighter weight.
In addition, the implementation was refactored to reduce complexity and values passed around
but not used.