Skip to content

ext/opcache/zend_shared_alloc: use memfd for locking if available #10589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

MaxKellermann
Copy link
Contributor

A memfd is a virtual file that has no reachable path, therefore does not clobber any filesystem. It is deleted automatically as soon as the last handle gets closed. The feature is available since Linux kernel 3.17.

A memfd is a virtual file that has no reachable path, therefore does
not clobber any filesystem.  It is deleted automatically as soon as
the last handle gets closed.  The feature is available since Linux
kernel 3.17.
@MaxKellermann
Copy link
Contributor Author

FreeBSD fails:

Tue Feb 14 17:32:15 2023 (47230): Error Cannot create lock - Bad file descriptor (9)

Apparently, this feature is incomplete on FreeBSD and unusable for us; I've added #ifdef __linux__.

@MaxKellermann
Copy link
Contributor Author

MACOS_DEBUG_NTS failure unrelated. Sigh.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iluuu1994 iluuu1994 requested a review from devnexen February 23, 2023 20:15
@devnexen
Copy link
Member

FreeBSD fails:

Tue Feb 14 17:32:15 2023 (47230): Error Cannot create lock - Bad file descriptor (9)

Apparently, this feature is incomplete on FreeBSD and unusable for us; I've added #ifdef __linux__.

Ok weird error ... well better disable it in this case indeed.

@devnexen devnexen closed this in e9c8621 Feb 23, 2023
@MaxKellermann MaxKellermann deleted the memfd_lock branch February 23, 2023 23:07
@staabm
Copy link
Contributor

staabm commented Feb 24, 2023

A memfd is a virtual file that has no reachable path, therefore does not clobber any filesystem. It is deleted automatically as soon as the last handle gets closed

is this a kind of lock-api which can be made available to userland php?

@MaxKellermann
Copy link
Contributor Author

is this a kind of lock-api which can be made available to userland php?

What do you mean? It would certainly possible to write a stream implementation backed by memfd, but what do you mean by "lock-api"? How does PHP code need any locking?

@staabm
Copy link
Contributor

staabm commented Feb 24, 2023

when implementing parallel processes/threads in php userland code you sometimes need primitives to handle some lock'ing.

e.g. https://symfony.com/doc/current/components/lock.html

my question was about, whether this kind of locks which are used here, can also be utilized from php-userland (as I read the PR here, these locks seem to be very lightweight)

@MaxKellermann
Copy link
Contributor Author

this kind of locks which are used here, can also be utilized from php-userland

Right now, this is implemented only inside the opcache extension, and is not exposed to PHP code, but it would be possible to add a PHP API for that. Though I wouldn't recommend doing that, because ....

as I read the PR here, these locks seem to be very lightweight

No, it's not at all. File locks are as heavy as locks can be. Don't.

Regular mutexes are lightweight (at least on Linux) because the non-contending common case is implemented without any system calls. Use a mutex, not file locks, if possible!

This is the very reason why I suggested not using file locks and submitted this PR which has been lingering for a month: #10277

@MaxKellermann
Copy link
Contributor Author

e.g. https://symfony.com/doc/current/components/lock.html

WTF, a locking library that depends on a logging library - talk about framework bloat ;-)

That library has a flock backend, but that requires a file path, just like opcache required one, prior to this PR.

As I said, once PHP exposes memfd_create to PHP code, any PHP code using flock could benefit from the advantages of memfd (auto-cleanup and no writable filesystem required; but performance is not improved by memfd flocks!) - though this is Linux-only and thus not portable.

The library also has a SysV semaphore backend. SysV semaphores are an ugly beast. They live in a global namespace, and if one application creates too many, the global limit will be exceeded and all other processes wanting to use them are broken. Remember the old times when a buggy Apache process could take everything down, and an Apache restart was impossible until you fiddled with ipcrm to delete the semaphores manually? Don't use semaphores, they belong in the trash can of history, an ill concept.

There are other implementations in Symfony like using PostgreSQL's locking capabilities - that looks like a "because we can" show-off implementation that has no practical use. Communicating forth and back with other processes over the network for locking stuff is the worst performance one can imagine.

There's no mutex implementation, probably because PHP provides no language access to mutexes? (Or does it?)

Mutexes, being the fastest way for locking (except for special-case stuff like spinlocks), have disadvantages, of course - they require shared memory, and if one process crashes, it may leave the mutex locked (or in an illegal state), breaking all other processes. File locks do not have that problem (the kernel cleans up after you because only the kernel manages all the state), but they are slower (because the kernel manages all the state, requiring context switches for all operations).

@iluuu1994
Copy link
Member

There are other implementations in Symfony like using PostgreSQL's locking capabilities - that looks like a "because we can" show-off implementation that has no practical use. Communicating forth and back with other processes over the network for locking stuff is the worst performance one can imagine.

This is useful for load-balanced applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants