From 29db9171bb2c94deb60c3a5290faa065bad349f4 Mon Sep 17 00:00:00 2001 From: Vadim Sadokhov <65451602+astrophysik@users.noreply.github.com> Date: Tue, 17 Oct 2023 16:00:43 +0300 Subject: [PATCH] reduce PhpWorker and PhpScript heap allocations (#895) add static storages for PhpWorker and PhpScript --- runtime/interface.cpp | 86 +++++++---------- runtime/interface.h | 4 +- server/job-workers/job-worker-server.cpp | 10 +- server/php-engine.cpp | 45 +++++---- server/php-engine.h | 5 +- server/php-queries.cpp | 10 +- server/php-query-data.cpp | 115 ----------------------- server/php-query-data.h | 52 +++------- server/php-runner.cpp | 29 +----- server/php-runner.h | 7 +- server/php-worker.cpp | 31 +++--- server/php-worker.h | 9 +- server/server.cmake | 1 - server/signal-handlers.cpp | 5 +- tests/cpp/runtime/_runtime-tests-env.cpp | 2 +- 15 files changed, 111 insertions(+), 300 deletions(-) delete mode 100644 server/php-query-data.cpp diff --git a/runtime/interface.cpp b/runtime/interface.cpp index 1e878d44d6..5a0432f01c 100644 --- a/runtime/interface.cpp +++ b/runtime/interface.cpp @@ -18,6 +18,7 @@ #include "common/algorithms/string-algorithms.h" #include "common/macos-ports.h" #include "common/tl/constants/common.h" +#include "common/wrappers/overloaded.h" #include "net/net-connections.h" #include "runtime/array_functions.h" @@ -402,19 +403,19 @@ void f$setcookie(const string &name, const string &value, int64_t expire, const } int64_t f$ignore_user_abort(Optional enable) { - php_assert(active_worker != nullptr && active_worker->conn != nullptr); + php_assert(php_worker.has_value() && php_worker->conn != nullptr); if (enable.is_null()) { return ignore_level; } else if (enable.val()) { - active_worker->conn->ignored = true; + php_worker->conn->ignored = true; return ignore_level++; } else { int prev = ignore_level > 0 ? ignore_level-- : 0; if (ignore_level == 0) { - active_worker->conn->ignored = false; + php_worker->conn->ignored = false; } - if (active_worker->conn->interrupted && !active_worker->conn->ignored) { - active_worker->conn->status = conn_error; + if (php_worker->conn->interrupted && !php_worker->conn->ignored) { + php_worker->conn->status = conn_error; f$exit(1); } return prev; @@ -558,13 +559,13 @@ static int ob_merge_buffers() { } void f$flush() { - php_assert(ob_cur_buffer >= 0 && active_worker != nullptr); + php_assert(ob_cur_buffer >= 0 && php_worker.has_value()); string_buffer const * http_body = compress_http_query_body(&oub[ob_system_level]); string_buffer const * http_headers = nullptr; - if (!active_worker->flushed_http_connection) { + if (!php_worker->flushed_http_connection) { http_headers = get_headers(); - active_worker->flushed_http_connection = true; + php_worker->flushed_http_connection = true; } http_send_immediate_response(http_headers ? http_headers->buffer() : nullptr, http_headers ? http_headers->size() : 0, http_body->buffer(), http_body->size()); @@ -574,7 +575,7 @@ void f$flush() { void f$fastcgi_finish_request(int64_t exit_code) { int const ob_total_buffer = ob_merge_buffers(); - if (active_worker != nullptr && active_worker->flushed_http_connection) { + if (php_worker.has_value() && php_worker->flushed_http_connection) { string const raw_response = oub[ob_total_buffer].str(); http_set_result(nullptr, 0, raw_response.c_str(), raw_response.size(), static_cast(exit_code)); php_assert (0); @@ -1544,8 +1545,8 @@ static void save_rpc_query_headers(const tl_query_header_t &header) { } } -static void init_superglobals(const http_query_data &http_data, const rpc_query_data &rpc_data, const job_query_data &job_data) { - rpc_parse(rpc_data.data, rpc_data.len); +static void init_superglobals_impl(const http_query_data &http_data, const rpc_query_data &rpc_data, const job_query_data &job_data) { + rpc_parse(rpc_data.data.data(), rpc_data.data.size()); reset_superglobals(); @@ -1735,10 +1736,10 @@ static void init_superglobals(const http_query_data &http_data, const rpc_query_ if (rpc_data.header.qid) { v$_SERVER.set_value(string("RPC_REQUEST_ID"), f$strval(static_cast(rpc_data.header.qid))); save_rpc_query_headers(rpc_data.header); - v$_SERVER.set_value(string("RPC_REMOTE_IP"), static_cast(rpc_data.ip)); - v$_SERVER.set_value(string("RPC_REMOTE_PORT"), static_cast(rpc_data.port)); - v$_SERVER.set_value(string("RPC_REMOTE_PID"), static_cast(rpc_data.pid)); - v$_SERVER.set_value(string("RPC_REMOTE_UTIME"), rpc_data.utime); + v$_SERVER.set_value(string("RPC_REMOTE_IP"), static_cast(rpc_data.remote_pid.ip)); + v$_SERVER.set_value(string("RPC_REMOTE_PORT"), static_cast(rpc_data.remote_pid.port)); + v$_SERVER.set_value(string("RPC_REMOTE_PID"), static_cast(rpc_data.remote_pid.pid)); + v$_SERVER.set_value(string("RPC_REMOTE_UTIME"), rpc_data.remote_pid.utime); } is_head_query = false; if (http_data.request_method_len) { @@ -1801,47 +1802,26 @@ static http_query_data empty_http_data; static rpc_query_data empty_rpc_data; static job_query_data empty_job_data; -void init_superglobals(php_query_data *data) { - http_query_data *http_data; - rpc_query_data *rpc_data; - job_query_data *job_data; - if (data != nullptr) { - if (data->rpc_data != nullptr) { - php_assert (data->http_data == nullptr); - php_assert (data->job_data == nullptr); +void init_superglobals(const php_query_data_t &data) { + // init superglobals depending on the request type + std::visit(overloaded{ + [](const rpc_query_data &rpc_data) { query_type = QUERY_TYPE_RPC; - - http_data = &empty_http_data; - rpc_data = data->rpc_data; - job_data = &empty_job_data; - } else if (data->http_data != nullptr) { - php_assert (data->rpc_data == nullptr); - php_assert (data->job_data == nullptr); + init_superglobals_impl(empty_http_data, rpc_data, empty_job_data); + }, + [](const http_query_data &http_data) { query_type = QUERY_TYPE_HTTP; - - http_data = data->http_data; - rpc_data = &empty_rpc_data; - job_data = &empty_job_data; - } else { - php_assert (data->job_data != nullptr); - php_assert (data->rpc_data == nullptr); - php_assert (data->http_data == nullptr); - + init_superglobals_impl(http_data, empty_rpc_data, empty_job_data); + }, + [](const job_query_data &job_data) { query_type = QUERY_TYPE_JOB; - - http_data = &empty_http_data; - rpc_data = &empty_rpc_data; - job_data = data->job_data; + init_superglobals_impl(empty_http_data, empty_rpc_data, job_data); + }, + [](const null_query_data &) { + query_type = QUERY_TYPE_CONSOLE; + init_superglobals_impl(empty_http_data, empty_rpc_data, empty_job_data); } - } else { - query_type = QUERY_TYPE_CONSOLE; - - http_data = &empty_http_data; - rpc_data = &empty_rpc_data; - job_data = &empty_job_data; - } - - init_superglobals(*http_data, *rpc_data, *job_data); + }, data); } double f$get_net_time() { @@ -2456,7 +2436,7 @@ void global_init_script_allocator() { dl::global_init_script_allocator(); } -void init_runtime_environment(php_query_data *data, void *mem, size_t script_mem_size, size_t oom_handling_mem_size) { +void init_runtime_environment(const php_query_data_t &data, void *mem, size_t script_mem_size, size_t oom_handling_mem_size) { dl::init_script_allocator(mem, script_mem_size, oom_handling_mem_size); reset_global_interface_vars(); init_runtime_libs(); diff --git a/runtime/interface.h b/runtime/interface.h index a1ed113e9d..9766f448aa 100644 --- a/runtime/interface.h +++ b/runtime/interface.h @@ -167,7 +167,7 @@ bool f$is_uploaded_file(const string &filename); bool f$move_uploaded_file(const string &oldname, const string &newname); -void init_superglobals(php_query_data *data); +void init_superglobals(const php_query_data_t &data); double f$get_net_time(); @@ -204,7 +204,7 @@ Optional> f$getopt(const string &options, array longopts = void global_init_runtime_libs(); void global_init_script_allocator(); -void init_runtime_environment(php_query_data *data, void *mem, size_t script_mem_size, size_t oom_handling_mem_size = 0); +void init_runtime_environment(const php_query_data_t &data, void *mem, size_t script_mem_size, size_t oom_handling_mem_size = 0); void free_runtime_environment(); diff --git a/server/job-workers/job-worker-server.cpp b/server/job-workers/job-worker-server.cpp index 2eb780466c..f054b46626 100644 --- a/server/job-workers/job-worker-server.cpp +++ b/server/job-workers/job-worker-server.cpp @@ -72,7 +72,7 @@ int jobs_server_php_wakeup(connection *c) { double timeout = worker->enter_lifecycle(); if (timeout == 0) { - delete worker; + php_worker.reset(); jobs_server_at_query_end(c); } else { assert(c->pending_queries >= 0 && c->status == conn_wait_net); @@ -173,10 +173,10 @@ int JobWorkerServer::job_parse_execute(connection *c) noexcept { job_stat.job_request_max_real_memory_used = job_memory_stats.max_real_memory_used; job_stat.job_request_max_memory_used = job_memory_stats.max_memory_used; - job_query_data *job_data = job_query_data_create(job, [](JobSharedMessage *job_response) { - return vk::singleton::get().send_job_reply(job_response); - }); - reinterpret_cast(c->custom_data)->worker = new PhpWorker(job_worker, c, nullptr, nullptr, job_data, job->job_id, left_job_time); + php_query_data_t job_data = job_query_data{job, [](JobSharedMessage *job_response) { + return vk::singleton::get().send_job_reply(job_response);}}; + php_worker.emplace(job_worker, c, std::move(job_data), job->job_id, left_job_time); + reinterpret_cast(c->custom_data)->worker = &php_worker.value(); set_connection_timeout(c, left_job_time); c->status = conn_wait_net; diff --git a/server/php-engine.cpp b/server/php-engine.cpp index 3cbd0d963d..4610dd7f08 100644 --- a/server/php-engine.cpp +++ b/server/php-engine.cpp @@ -314,7 +314,7 @@ command_t *create_command_net_writer(const char *data, int data_len, command_t * int run_once_count = 1; int queries_to_recreate_script = 100; -PhpScript *php_script; +std::optional php_script; int has_pending_scripts() { return php_worker_run_flag || pending_http_queue.first_query != (conn_query *)&pending_http_queue; @@ -354,14 +354,14 @@ void on_net_event(int event_status) { if (event_status == 0) { return; } - assert (active_worker != nullptr); + assert (php_worker.has_value()); if (event_status < 0) { - active_worker->terminate(0, script_error_t::net_event_error, "memory limit on net event"); - active_worker->wakeup(); + php_worker->terminate(0, script_error_t::net_event_error, "memory limit on net event"); + php_worker->wakeup(); return; } - if (active_worker->waiting) { - active_worker->wakeup(); + if (php_worker->waiting) { + php_worker->wakeup(); } } @@ -525,7 +525,7 @@ int do_hts_func_wakeup(connection *c, bool flag) { assert(worker); double timeout = worker->enter_lifecycle(); if (timeout == 0) { - delete worker; + php_worker.reset(); hts_at_query_end(c, flag); } else { assert (timeout > 0); @@ -613,14 +613,13 @@ int hts_func_execute(connection *c, int op) { } /** save query here **/ - http_query_data *http_data = http_query_data_create(qUri, qUriLen, qGet, qGetLen, qHeaders, qHeadersLen, qPost, - qPostLen, query_type_str, D->query_flags & QF_KEEPALIVE, - inet_sockaddr_address(&c->remote_endpoint), - inet_sockaddr_port(&c->remote_endpoint)); + php_query_data_t http_data = http_query_data{qUri, qGet, qHeaders, qPost, query_type_str, + qUriLen, qGetLen, qHeadersLen, qPostLen, static_cast(strlen(query_type_str)), + D->query_flags & QF_KEEPALIVE, inet_sockaddr_address(&c->remote_endpoint), inet_sockaddr_port(&c->remote_endpoint)}; static long long http_script_req_id = 0; - PhpWorker *worker = new PhpWorker(http_worker, c, http_data, nullptr, nullptr, ++http_script_req_id, script_timeout); - D->extra = worker; + php_worker.emplace(http_worker, c, std::move(http_data), ++http_script_req_id, script_timeout); + D->extra = &php_worker.value(); set_connection_timeout(c, script_timeout); c->status = conn_wait_net; @@ -640,7 +639,7 @@ int hts_func_close(connection *c, int who __attribute__((unused))) { double timeout = worker->enter_lifecycle(); D->extra = nullptr; assert ("worker is unfinished after closing connection" && timeout == 0); - delete worker; + php_worker.reset(); } return 0; } @@ -825,7 +824,7 @@ int rpcx_func_wakeup(connection *c) { assert(worker); double timeout = worker->enter_lifecycle(); if (timeout == 0) { - delete worker; + php_worker.reset(); rpcx_at_query_end(c); } else { assert (c->pending_queries >= 0 && c->status == conn_wait_net); @@ -844,7 +843,7 @@ int rpcx_func_close(connection *c, int who __attribute__((unused))) { double timeout = worker->enter_lifecycle(); D->extra = nullptr; assert ("worker is unfinished after closing connection" && timeout == 0); - delete worker; + php_worker.reset(); if (!has_pending_scripts()) { lease_set_ready(); @@ -984,7 +983,7 @@ int rpcx_execute(connection *c, int op, raw_message *raw) { int64_t left_bytes_without_headers = tl_fetch_unread(); len -= (left_bytes_with_headers - left_bytes_without_headers); - assert(len % 4 == 0); + assert(len % sizeof(int) == 0); long long req_id = header.qid; @@ -1010,19 +1009,17 @@ int rpcx_execute(connection *c, int op, raw_message *raw) { double actual_script_timeout = custom_settings.has_timeout() ? normalize_script_timeout(custom_settings.php_timeout_ms / 1000.0) : script_timeout; set_connection_timeout(c, actual_script_timeout); - char buf[len + 1]; - auto fetched_bytes = tl_fetch_data(buf, len); + std::vector buffer(len / sizeof(int)); + auto fetched_bytes = tl_fetch_data(buffer.data(), len); if (fetched_bytes == -1) { client_rpc_error(c, req_id, tl_fetch_error_code(), tl_fetch_error_string()); return 0; } assert(fetched_bytes == len); auto *D = TCP_RPC_DATA(c); - rpc_query_data *rpc_data = rpc_query_data_create(std::move(header), reinterpret_cast(buf), len / static_cast(sizeof(int)), D->remote_pid.ip, - D->remote_pid.port, D->remote_pid.pid, D->remote_pid.utime); - - PhpWorker *worker = new PhpWorker(run_once ? once_worker : rpc_worker, c, nullptr, rpc_data, nullptr, req_id, actual_script_timeout); - D->extra = worker; + php_query_data_t rpc_data = rpc_query_data{std::move(header), std::move(buffer), D->remote_pid}; + php_worker.emplace(run_once ? once_worker : rpc_worker, c, std::move(rpc_data), req_id, actual_script_timeout); + D->extra = &php_worker.value(); c->status = conn_wait_net; rpcx_func_wakeup(c); diff --git a/server/php-engine.h b/server/php-engine.h index 16a258131a..82e25b2202 100644 --- a/server/php-engine.h +++ b/server/php-engine.h @@ -4,6 +4,8 @@ #pragma once +#include + #include "common/sanitizer.h" #include "server/php-queries.h" @@ -56,7 +58,8 @@ extern int run_once_count; extern int queries_to_recreate_script; class PhpScript; -extern PhpScript *php_script; + +extern std::optional php_script; void turn_sigterm_on(); diff --git a/server/php-queries.cpp b/server/php-queries.cpp index e6ff1ae581..ad3d24d95d 100644 --- a/server/php-queries.cpp +++ b/server/php-queries.cpp @@ -946,11 +946,11 @@ void db_run_query(int host_num, const char *request, int request_len, int timeou } void http_send_immediate_response(const char *headers, int headers_len, const char *body, int body_len) { - php_assert(active_worker != nullptr); - if (active_worker->mode == http_worker) { - write_out(&active_worker->conn->Out, headers, headers_len); - write_out(&active_worker->conn->Out, body, body_len); - flush_connection_output(active_worker->conn); + php_assert(php_worker.has_value()); + if (php_worker->mode == http_worker) { + write_out(&php_worker->conn->Out, headers, headers_len); + write_out(&php_worker->conn->Out, body, body_len); + flush_connection_output(php_worker->conn); } else { php_warning("Immediate HTTP response available only from HTTP worker"); } diff --git a/server/php-query-data.cpp b/server/php-query-data.cpp deleted file mode 100644 index 1abe45602a..0000000000 --- a/server/php-query-data.cpp +++ /dev/null @@ -1,115 +0,0 @@ -// Compiler for PHP (aka KPHP) -// Copyright (c) 2020 LLC «V Kontakte» -// Distributed under the GPL v3 License, see LICENSE.notice.txt - -#include "server/php-query-data.h" - -#include -#include -#include -#include -#include - -#include "common/dl-utils-lite.h" - -static void *memdup(const void *src, size_t x) { - if (!src) { - assert(!x); - return nullptr; - } - void *res = malloc(x); - assert(res); - memcpy(res, src, x); - return res; -} - -http_query_data *http_query_data_create( - const char *qUri, int qUriLen, - const char *qGet, int qGetLen, - const char *qHeaders, int qHeadersLen, - const char *qPost, int qPostLen, - const char *request_method, int keep_alive, unsigned int ip, unsigned int port) { - http_query_data *d = (http_query_data *)malloc(sizeof(http_query_data)); - - //TODO remove memdup completely. We can just copy pointers - d->uri = (char *)memdup(qUri, qUriLen); - d->get = (char *)memdup(qGet, qGetLen); - d->headers = (char *)memdup(qHeaders, qHeadersLen); - if (qPost) { - d->post = (char *)memdup(qPost, qPostLen); - } else { - d->post = nullptr; - } - d->uri_len = qUriLen; - d->get_len = qGetLen; - d->headers_len = qHeadersLen; - d->post_len = qPostLen; - - d->request_method = (char *)memdup(request_method, strlen(request_method)); - d->request_method_len = (int)strlen(request_method); - - d->keep_alive = keep_alive; - - d->ip = ip; - d->port = port; - - return d; -} - -void http_query_data_free(http_query_data *d) { - if (d == nullptr) { - return; - } - - free(d->uri); - free(d->get); - free(d->headers); - free(d->post); - - free(d->request_method); - - free(d); -} - -rpc_query_data *rpc_query_data_create(tl_query_header_t &&header, int *data, int len, unsigned int ip, short port, short pid, int utime) { - auto *d = new rpc_query_data; - - d->header = std::move(header); - - d->data = static_cast(memdup(data, sizeof(int) * len)); - d->len = len; - - d->ip = ip; - d->port = port; - d->pid = pid; - d->utime = utime; - - return d; -} - -void rpc_query_data_free(rpc_query_data *d) { - if (d == nullptr) { - return; - } - - free(d->data); - delete d; -} - -php_query_data *php_query_data_create(http_query_data *http_data, rpc_query_data *rpc_data, job_query_data *job_data) { - php_query_data *d = (php_query_data *)malloc(sizeof(php_query_data)); - - d->http_data = http_data; - d->rpc_data = rpc_data; - d->job_data = job_data; - - return d; -} - -void php_query_data_free(php_query_data *d) { - http_query_data_free(d->http_data); - rpc_query_data_free(d->rpc_data); - job_query_data_free(d->job_data); - - free(d); -} diff --git a/server/php-query-data.h b/server/php-query-data.h index 96577ba71d..387c537de0 100644 --- a/server/php-query-data.h +++ b/server/php-query-data.h @@ -4,37 +4,27 @@ #pragma once +#include +#include + #include "common/tl/query-header.h" +#include "common/dl-utils-lite.h" + -/** http_query_data **/ struct http_query_data { - char *uri, *get, *headers, *post, *request_method; + char const *uri, *get, *headers, *post, *request_method; int uri_len, get_len, headers_len, post_len, request_method_len; int keep_alive; unsigned int ip; unsigned int port; }; -http_query_data *http_query_data_create(const char *qUri, int qUriLen, const char *qGet, int qGetLen, const char *qHeaders, - int qHeadersLen, const char *qPost, int qPostLen, const char *request_method, int keep_alive, unsigned int ip, unsigned int port); -void http_query_data_free(http_query_data *d); - -/** rpc_query_data **/ struct rpc_query_data { tl_query_header_t header; - - int *data, len; - - /** PID **/ - unsigned ip; - short port; - short pid; - int utime; + std::vector data; + process_id remote_pid; }; -rpc_query_data *rpc_query_data_create(tl_query_header_t &&header, int *data, int len, unsigned int ip, short port, short pid, int utime); -void rpc_query_data_free(rpc_query_data *d); - namespace job_workers { struct JobSharedMessage; } // namespace job_workers @@ -44,25 +34,9 @@ struct job_query_data { const char *(*send_reply) (job_workers::JobSharedMessage *job_response); }; -inline job_query_data *job_query_data_create(job_workers::JobSharedMessage *job_request, const char *(*send_reply) (job_workers::JobSharedMessage *job_response)) { - return new job_query_data{job_request, send_reply}; -} - -inline void job_query_data_free(job_query_data *job_data) { - if (job_data == nullptr) { - return; - } - delete job_data; -} - -/** php_query_data **/ -struct php_query_data { - http_query_data *http_data; - rpc_query_data *rpc_data; - job_query_data *job_data; -}; - -php_query_data *php_query_data_create(http_query_data *http_data, rpc_query_data *rpc_data, job_query_data *job_data); -void php_query_data_free(php_query_data *d); - +using null_query_data = std::monostate; +using php_query_data_t = std::variant; diff --git a/server/php-runner.cpp b/server/php-runner.cpp index 0416635e70..e1065953ee 100644 --- a/server/php-runner.cpp +++ b/server/php-runner.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -155,7 +156,7 @@ PhpScript::~PhpScript() noexcept { munmap(run_mem, mem_size); } -void PhpScript::init(script_t *script, php_query_data *data_to_set) noexcept { +void PhpScript::init(script_t *script, php_query_data_t *data_to_set) noexcept { assert (script != nullptr); assert_state(run_state_t::empty); @@ -337,8 +338,8 @@ void PhpScript::finish() noexcept { static char buf[buf_size]; buf[0] = 0; if (disable_access_log < 2) { - if (data != nullptr) { - http_query_data *http_data = data->http_data; + if (data != nullptr && std::holds_alternative(*data)) { + http_query_data *http_data = &std::get(*data); if (http_data != nullptr) { if (disable_access_log) { snprintf(buf, buf_size, "[uri = %.*s?]", min(http_data->uri_len, 200), http_data->uri); @@ -406,33 +407,13 @@ void PhpScript::query_answered() noexcept { } void PhpScript::run() noexcept { - if (data != nullptr) { - http_query_data *http_data = data->http_data; - if (http_data != nullptr) { - //fprintf (stderr, "arguments\n"); - //fprintf (stderr, "[uri = %.*s]\n", http_data->uri_len, http_data->uri); - //fprintf (stderr, "[get = %.*s]\n", http_data->get_len, http_data->get); - //fprintf (stderr, "[headers = %.*s]\n", http_data->headers_len, http_data->headers); - //fprintf (stderr, "[post = %.*s]\n", http_data->post_len, http_data->post); - } - - rpc_query_data *rpc_data = data->rpc_data; - if (rpc_data != nullptr) { - /* - fprintf (stderr, "N = %d\n", rpc_data->len); - for (int i = 0; i < rpc_data->len; i++) { - fprintf (stderr, "%d: %10d\n", i, rpc_data->data[i]); - } - */ - } - } assert (run_main->run != nullptr); dl::enter_critical_section(); in_script_context = true; auto oom_handling_memory_size = static_cast(std::ceil(mem_size * oom_handling_memory_ratio)); auto script_memory_size = mem_size - oom_handling_memory_size; - init_runtime_environment(data, run_mem, script_memory_size, oom_handling_memory_size); + init_runtime_environment(*data, run_mem, script_memory_size, oom_handling_memory_size); dl::leave_critical_section(); php_assert (dl::in_critical_section == 0); // To ensure that no critical section is left at the end of the initialization check_net_context_errors(); diff --git a/server/php-runner.h b/server/php-runner.h index 79df7ed120..5aa0fa2392 100644 --- a/server/php-runner.h +++ b/server/php-runner.h @@ -4,6 +4,8 @@ #pragma once +#include + #include "common/dl-utils-lite.h" #include "common/kprintf.h" #include "common/mixin/not_copyable.h" @@ -122,7 +124,8 @@ class PhpScript { ucontext_t_portable run_context{}; script_t *run_main{nullptr}; - php_query_data *data{nullptr}; + //logically it's a reference but since we initialize this value in init() it has a pointer type + php_query_data_t *data{nullptr}; script_result *res{nullptr}; static void script_context_entrypoint() noexcept; @@ -134,7 +137,7 @@ class PhpScript { void try_run_shutdown_functions_on_timeout() noexcept; void check_net_context_errors() noexcept; - void init(script_t *script, php_query_data *data_to_set) noexcept; + void init(script_t *script, php_query_data_t *data_to_set) noexcept; void pause() noexcept; void ask_query(php_query_base_t *q) noexcept; diff --git a/server/php-worker.cpp b/server/php-worker.cpp index e199819d55..dc13dadb45 100644 --- a/server/php-worker.cpp +++ b/server/php-worker.cpp @@ -3,6 +3,7 @@ // Distributed under the GPL v3 License, see LICENSE.notice.txt #include +#include #include #include "common/precise-time.h" @@ -24,7 +25,7 @@ #include "server/php-worker.h" #include "server/server-stats.h" -PhpWorker *active_worker = nullptr; +std::optional php_worker; double PhpWorker::enter_lifecycle() noexcept { if (finish_time < precise_now + 0.01) { @@ -33,7 +34,7 @@ double PhpWorker::enter_lifecycle() noexcept { on_wakeup(); tvkprintf(php_runner, 3, "PHP-worker enter lifecycle [php-script state = %d, conn status = %d] lifecycle [req_id = %016llx]\n", - php_script ? static_cast(php_script->state) : -1, conn->status, req_id); + php_script.has_value() ? static_cast(php_script->state) : -1, conn->status, req_id); paused = false; do { switch (state) { @@ -67,7 +68,7 @@ double PhpWorker::enter_lifecycle() noexcept { } while (!paused); tvkprintf(php_runner, 3, "PHP-worker [php-script state = %d, conn status = %d] return in net reactor [req_id = %016llx]\n", - php_script ? static_cast(php_script->state) : -1, conn->status, req_id); + php_script.has_value() ? static_cast(php_script->state) : -1, conn->status, req_id); assert(conn->status == conn_wait_net); return get_timeout(); } @@ -145,8 +146,6 @@ void PhpWorker::state_init_script() noexcept { get_utime_monotonic(); start_time = precise_now; - assert(active_worker == nullptr); - active_worker = this; vk::singleton::get().set_running_worker_status(); // init memory allocator for queries @@ -154,11 +153,11 @@ void PhpWorker::state_init_script() noexcept { script_t *script = get_script(); dl_assert(script != nullptr, "failed to get script"); - if (php_script == nullptr) { - php_script = new PhpScript(max_memory, oom_handling_memory_ratio, 8 << 20); + if (!php_script.has_value()) { + php_script.emplace(max_memory, oom_handling_memory_ratio, 8 << 20); } dl::init_critical_section(); - php_script->init(script, data); + php_script->init(script, &data); php_script->set_timeout(timeout); state = phpq_run; } @@ -393,8 +392,6 @@ void PhpWorker::state_free_script() noexcept { php_worker_run_flag = 0; int f = 0; - assert(active_worker == this); - active_worker = nullptr; vk::singleton::get().set_idle_worker_status(); if (mode == once_worker) { static int left = run_once_count; @@ -416,9 +413,8 @@ void PhpWorker::state_free_script() noexcept { static int finished_queries = 0; if ((++finished_queries) % queries_to_recreate_script == 0 - || (!use_madvise_dontneed && php_script->memory_get_total_usage() > memory_used_to_recreate_script)) { - delete php_script; - php_script = nullptr; + || (!use_madvise_dontneed && php_script.value().memory_get_total_usage() > memory_used_to_recreate_script)) { + php_script.reset(); finished_queries = 0; } @@ -442,10 +438,10 @@ double PhpWorker::get_timeout() const noexcept { return time_left; } -PhpWorker::PhpWorker(php_worker_mode_t mode_, connection *c, http_query_data *http_data, rpc_query_data *rpc_data, job_query_data *job_data, +PhpWorker::PhpWorker(php_worker_mode_t mode_, connection *c, php_query_data_t query_data, long long int req_id_, double timeout) : conn(c) - , data(php_query_data_create(http_data, rpc_data, job_data)) + , data(std::move(query_data)) , paused(false) , flushed_http_connection(false) , terminate_flag(false) @@ -469,8 +465,3 @@ PhpWorker::PhpWorker(php_worker_mode_t mode_, connection *c, http_query_data *ht } tvkprintf(php_runner, 1, "initialize PHP-worker [req_id = %016llx]\n", req_id); } - -PhpWorker::~PhpWorker() { - php_query_data_free(data); - data = nullptr; -} diff --git a/server/php-worker.h b/server/php-worker.h index 276f1cf502..48cf690901 100644 --- a/server/php-worker.h +++ b/server/php-worker.h @@ -35,7 +35,7 @@ class PhpWorker { public: struct connection *conn; - php_query_data *data; + php_query_data_t data; bool paused; bool flushed_http_connection; @@ -58,9 +58,8 @@ class PhpWorker { long long req_id; int target_fd; - PhpWorker(php_worker_mode_t mode_, connection *c, http_query_data *http_data, rpc_query_data *rpc_data, job_query_data *job_data, - long long req_id_, double timeout); - ~PhpWorker(); + PhpWorker(php_worker_mode_t mode_, connection *c, php_query_data_t php_query_data, long long req_id_, double timeout); + ~PhpWorker() = default; double enter_lifecycle() noexcept; @@ -82,4 +81,4 @@ class PhpWorker { void state_finish() noexcept; }; -extern PhpWorker *active_worker; +extern std::optional php_worker; diff --git a/server/server.cmake b/server/server.cmake index 257c82bf07..9e2f22ebe9 100644 --- a/server/server.cmake +++ b/server/server.cmake @@ -18,7 +18,6 @@ prepend(KPHP_SERVER_SOURCES ${BASE_DIR}/server/ php-mc-connections.cpp php-queries.cpp php-queries-types.cpp - php-query-data.cpp php-runner.cpp php-init-scripts.cpp php-sql-connections.cpp diff --git a/server/signal-handlers.cpp b/server/signal-handlers.cpp index d274ad68b5..97ee71b407 100644 --- a/server/signal-handlers.cpp +++ b/server/signal-handlers.cpp @@ -120,8 +120,8 @@ void print_http_data() { } if (!PhpScript::current_script) { write_str(2, "\nPHPScriptBase::current_script is nullptr\n"); - } else if (PhpScript::current_script->data) { - if (http_query_data *data = PhpScript::current_script->data->http_data) { + } else if (PhpScript::current_script->data != nullptr && std::holds_alternative(*PhpScript::current_script->data)) { + http_query_data *data = &std::get(*PhpScript::current_script->data); write_str(2, "\nuri\n"); write(2, data->uri, data->uri_len); write_str(2, "\nget\n"); @@ -132,7 +132,6 @@ void print_http_data() { if (data->post && data->post_len > 0) { write(2, data->post, data->post_len); } - } } } diff --git a/tests/cpp/runtime/_runtime-tests-env.cpp b/tests/cpp/runtime/_runtime-tests-env.cpp index a073d0e420..15228c518b 100644 --- a/tests/cpp/runtime/_runtime-tests-env.cpp +++ b/tests/cpp/runtime/_runtime-tests-env.cpp @@ -30,7 +30,7 @@ class RuntimeTestsEnvironment final : public testing::Environment { global_init_runtime_libs(); global_init_script_allocator(); - init_runtime_environment(nullptr, script_memory, script_memory_size); + init_runtime_environment(null_query_data{}, script_memory, script_memory_size); php_disable_warnings = true; php_warning_level = 0; }