Skip to content

Add API to exempt function from being traced in JIT #15559

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 24, 2024
Merged

Add API to exempt function from being traced in JIT #15559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e0a40f4
Add API to exempt function from being JITed
bwoebi Jul 22, 2024
d4fdaa6
Add check for blacklisted traces when calling functions
bwoebi Sep 2, 2024
8e067ce
Allow EG(vm_interrupt) in observer start handler to leave JIT
bwoebi Sep 2, 2024
e9bb403
Also reset hot handlers
bwoebi Sep 2, 2024
4349069
Use ZEND_FUNC_JIT_ON_HOT_TRACE flag
bwoebi Sep 16, 2024
498f6ae
Fix after rebase
bwoebi Sep 16, 2024
6e90810
Fixup remaining place in zend_jit_vm_helpers.c
bwoebi Sep 20, 2024
ef5500f
Add NEWS/UPGRADING entries
bwoebi Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ PHP NEWS

- Opcache:
. Fixed bug GH-15657 (Segmentation fault in dasm_x86.h). (nielsdos)
. Added opcache_jit_blacklist() function. (Bob)

- PHPDBG:
. Fixed bug GH-15901 (phpdbg: Assertion failure on i funcs). (cmb)
Expand Down
4 changes: 4 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,10 @@ PHP 8.4 UPGRADE NOTES
. Added mb_ucfirst and mb_lcfirst functions.
RFC: https://wiki.php.net/rfc/mb_ucfirst

- OPCache:
. Added opcache_jit_blacklist function. It allows skipping the tracing JIT
execution of select functions.

- PCNTL:
. Added pcntl_setns allowing a process to be reassociated with a namespace in order
to share resources with other processes within this context.
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/jit/zend_jit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ static bool zend_may_be_dynamic_property(zend_class_entry *ce, zend_string *memb
# endif
#endif

void zend_jit_status(zval *ret)
ZEND_EXT_API void zend_jit_status(zval *ret)
{
zval stats;
array_init(&stats);
Expand Down
3 changes: 2 additions & 1 deletion ext/opcache/jit/zend_jit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ void zend_jit_startup(void *jit_buffer, size_t size, bool reattached);
void zend_jit_shutdown(void);
void zend_jit_activate(void);
void zend_jit_deactivate(void);
void zend_jit_status(zval *ret);
ZEND_EXT_API void zend_jit_status(zval *ret);
ZEND_EXT_API void zend_jit_blacklist_function(zend_op_array *op_array);
Comment on lines -165 to +166
Copy link
Member

Choose a reason for hiding this comment

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

Make a normal API not a yet another hack for your observer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand what you mean? What would "a normal API" look like?

Copy link
Member

Choose a reason for hiding this comment

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

  • Make it work through use-level opcache_jit_blacklist() function (similar opcache_invalidate) or use "no-jit" PHP attribute, etc
  • Describe the API (behavior should not be driven by implementation details, it should satisfy the requirements).

void zend_jit_restart(void);

#define ZREG_LOAD (1<<0)
Expand Down
14 changes: 14 additions & 0 deletions ext/opcache/jit/zend_jit_ir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10102,6 +10102,20 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen
ir_STORE(jit_EX(opline), jit_IP(jit));
}
jit_observer_fcall_begin(jit, rx, observer_handler);

if (trace) {
int32_t exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_TO_VM);

exit_addr = zend_jit_trace_get_exit_addr(exit_point);
if (!exit_addr) {
return 0;
}
} else {
exit_addr = NULL;
}

zend_jit_check_timeout(jit, NULL /* we're inside the called function */, exit_addr);

jit_observer_fcall_is_unobserved_end(jit, &unobserved_data);
}

Expand Down
18 changes: 18 additions & 0 deletions ext/opcache/jit/zend_jit_trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7656,6 +7656,24 @@ static void zend_jit_blacklist_root_trace(const zend_op *opline, size_t offset)
zend_shared_alloc_unlock();
}

ZEND_EXT_API void zend_jit_blacklist_function(zend_op_array *op_array) {
zend_jit_op_array_trace_extension *jit_extension = (zend_jit_op_array_trace_extension *)ZEND_FUNC_INFO(op_array);
if (!jit_extension || !(jit_extension->func_info.flags & ZEND_FUNC_JIT_ON_HOT_TRACE)) {
return;
}

zend_shared_alloc_lock();
SHM_UNPROTECT();
Comment on lines +7665 to +7666
Copy link
Member

Choose a reason for hiding this comment

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

ZendAccelerator.c first do this in opposite order. First SHM_UNPROTECT() then zend_shared_alloc_lock().
This was done to allow usage of spin-locks. I'm not sure if this makes any difference, but it's better to use the same order everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm copying the logic from zend_jit_blacklist_root_trace where it's first alloc_lock, then SHM_UNPROTECT?

Copy link
Member

Choose a reason for hiding this comment

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

OK. This is not a big problem.

zend_jit_unprotect();

zend_jit_stop_persistent_op_array(op_array);
jit_extension->func_info.flags &= ~ZEND_FUNC_JIT_ON_HOT_TRACE;

zend_jit_protect();
SHM_PROTECT();
zend_shared_alloc_unlock();
}

static bool zend_jit_trace_is_bad_root(const zend_op *opline, zend_jit_trace_stop stop, size_t offset)
{
const zend_op **cache_opline = JIT_G(bad_root_cache_opline);
Expand Down
24 changes: 13 additions & 11 deletions ext/opcache/jit/zend_jit_vm_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,16 +521,17 @@ static int zend_jit_trace_record_fake_init_call_ex(zend_execute_data *call, zend
&& (func->op_array.fn_flags & (ZEND_ACC_CLOSURE|ZEND_ACC_FAKE_CLOSURE))) {
return -1;
}
if (func->type == ZEND_USER_FUNCTION
&& (func->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
if (func->type == ZEND_USER_FUNCTION) {
jit_extension =
(zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(&func->op_array);
if (UNEXPECTED(!jit_extension
|| !(jit_extension->func_info.flags & ZEND_FUNC_JIT_ON_HOT_TRACE)
|| (func->op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE))) {
if (UNEXPECTED(!jit_extension && (func->op_array.fn_flags & ZEND_ACC_CLOSURE))
|| (jit_extension && !(jit_extension->func_info.flags & ZEND_FUNC_JIT_ON_HOT_TRACE))
|| (func->op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE)) {
return -1;
}
func = (zend_function*)jit_extension->op_array;
if (func->op_array.fn_flags & ZEND_ACC_CLOSURE) {
func = (zend_function*)jit_extension->op_array;
}
}
if (is_megamorphic == ZEND_JIT_EXIT_POLYMORPHISM
/* TODO: use more accurate check ??? */
Expand Down Expand Up @@ -1100,17 +1101,18 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
stop = ZEND_JIT_TRACE_STOP_BAD_FUNC;
break;
}
if (func->type == ZEND_USER_FUNCTION
&& (func->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
if (func->type == ZEND_USER_FUNCTION) {
jit_extension =
(zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(&func->op_array);
if (UNEXPECTED(!jit_extension)
|| !(jit_extension->func_info.flags & ZEND_FUNC_JIT_ON_HOT_TRACE)
if (UNEXPECTED(!jit_extension && (func->op_array.fn_flags & ZEND_ACC_CLOSURE))
|| (jit_extension && !(jit_extension->func_info.flags & ZEND_FUNC_JIT_ON_HOT_TRACE))
|| (func->op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE)) {
stop = ZEND_JIT_TRACE_STOP_INTERPRETER;
break;
}
func = (zend_function*)jit_extension->op_array;
if (func->op_array.fn_flags & ZEND_ACC_CLOSURE) {
func = (zend_function*)jit_extension->op_array;
}
}

#ifndef HAVE_GCC_GLOBAL_REGS
Expand Down
2 changes: 2 additions & 0 deletions ext/opcache/opcache.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ function opcache_compile_file(string $filename): bool {}

function opcache_invalidate(string $filename, bool $force = false): bool {}

function opcache_jit_blacklist(Closure $closure): void {}

/**
* @return array<string, mixed>|false
* @refcount 1
Expand Down
8 changes: 7 additions & 1 deletion ext/opcache/opcache_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions ext/opcache/tests/jit/opcache_jit_blacklist.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
Basic usage of opcache_jit_blacklist()
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.protect_memory=1
opcache.jit=tracing
--EXTENSIONS--
opcache
--FILE--
<?php
function foo() {
$x = 1;
$x += 0;
++$x;
var_dump($x);
}
opcache_jit_blacklist(foo(...));
foo();
?>
--EXPECT--
int(2)
16 changes: 16 additions & 0 deletions ext/opcache/zend_accelerator_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "php.h"
#include "ZendAccelerator.h"
#include "zend_API.h"
#include "zend_closures.h"
#include "zend_shared_alloc.h"
#include "zend_accelerator_blacklist.h"
#include "php_ini.h"
Expand Down Expand Up @@ -924,6 +925,21 @@ ZEND_FUNCTION(opcache_invalidate)
}
}

/* {{{ Prevents JIT on function. Call it before the first invocation of the given function. */
ZEND_FUNCTION(opcache_jit_blacklist)
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to have a least one test for this function.
I suppose, it can't be called at this moment... :)

There is a big question about usability of this API.
It can prevent and revert JIT-ing of already cached functions, but it can't disable setting JIT counters at the first place.

Copy link
Member Author

@bwoebi bwoebi Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It can be called, but not really tested for its functionality, unless you have a way how to check whether a function is actually executed under jit without looking at it in gdb.
So added a very basic test, just calling it in a JIT scenario, to make sure it doesn't crash. Manually verified it works, too.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry, my mistake. I forgot about automatic registration through opcache.stub.php.

{
zval *closure;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &closure, zend_ce_closure) == FAILURE) {
RETURN_THROWS();
}

const zend_function *func = zend_get_closure_method_def(Z_OBJ_P(closure));
if (ZEND_USER_CODE(func->type)) {
zend_jit_blacklist_function((zend_op_array *)&func->op_array);
}
}

ZEND_FUNCTION(opcache_compile_file)
{
zend_string *script_name;
Expand Down
Loading