Revert "ext/opcache: use C11 atomics for "restart_in" (#10276)"

[MaxKellermann]

This reverts commit 061fcdb.

While the commit does indeed improve performance, @dstogov complained
that it disables the code path calling kill_all_lockers(), and thus
hanging workers are never killed and restarted.
#10276 (comment)

The fact that this feature was not implemented in the existing atomic
code path (i.e. Windows) did not mean that the feature was considered
not strictly necessary, but that the Windows implementation was
incomplete and broken.
#10276 (comment)

[iluuu1994]

Thank you!

[iluuu1994]

As per comment #10276 (comment) this doesn't seem to be accurate.

[MaxKellermann]

Yes, that explains it, thanks @cmb69 - I'll adjust the comment.

[MaxKellermann]

After @cmb69's explanation, I've adjusted the comment to explain why kill_all_lockers() is not needed on Windows.

[MaxKellermann]

I was just wondering if we can keep the C11 atomics if there is no SAPI that manages child processes. flock() has a huge amount of overhead compared to atomics.
(I don't care much if upstream uses atomics - we use our own PHP fork that keeps using atomics and pthread mutexes; has been for 2 years.)

[dstogov]

Looking into your reverted patch, I'm not sure if the usage of atomic_load() guarantees atomarity of updates. INCREMENT/DECREMENT were done with non atomic operations and without locks.

In the current opcache implementation flock() is executed once per request, so it shouldn't make a big performance penalty.

I would welcome a cheaper kind of locks, but they should come without breaking anything (may be wrapped with #ifdefs to allow easy switch between old and new implementations).

[MaxKellermann]

Looking into your reverted patch, I'm not sure if the usage of atomic_load() guarantees atomarity of updates.

My C11 code has the same guarantees as the old Windows specific code. If you believe that's wrong, please explain why.

INCREMENT/DECREMENT were done with non atomic operations and without locks.

Not true.

"Built-in increment and decrement operators and compound assignment are read-modify-write atomic operations with total sequentially consistent ordering (as if using memory_order_seq_cst)."

(I had to look it up before I wrote this patch - I'm a C++ guy, and never cared about C11.)

[dstogov]

INCREMENT/DECREMENT were done with non atomic operations and without locks.

Not true.

"Built-in increment and decrement operators and compound assignment are read-modify-write atomic operations with total sequentially consistent ordering (as if using memory_order_seq_cst)."

Yes, you are right (I wasn't sure).

[iluuu1994]

@MaxKellermann Thank you!