You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2016/10/29 22:17:12 UTC
svn commit: r1767128 - in /httpd/httpd/trunk: CHANGES
modules/http2/h2_conn.c modules/http2/h2_conn.h modules/http2/h2_mplx.c
modules/http2/h2_ngn_shed.c modules/http2/h2_session.c
modules/http2/h2_session.h modules/http2/h2_task.c
Author: icing
Date: Sat Oct 29 22:17:11 2016
New Revision: 1767128
URL: http://svn.apache.org/viewvc?rev=1767128&view=rev
Log:
mod_http2: earlier slave connection allocator destroy, code cleanups
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http2/h2_conn.c
httpd/httpd/trunk/modules/http2/h2_conn.h
httpd/httpd/trunk/modules/http2/h2_mplx.c
httpd/httpd/trunk/modules/http2/h2_ngn_shed.c
httpd/httpd/trunk/modules/http2/h2_session.c
httpd/httpd/trunk/modules/http2/h2_session.h
httpd/httpd/trunk/modules/http2/h2_task.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Oct 29 22:17:11 2016
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_http2: allocators from slave connections is released earlier, resulting
+ in less overall memory use on busy, long lived connections.
+ [Stefan Eissing]
+
*) mpm_unix: Apache fails to start if previously crashed then restarted with
the same PID (e.g. in container). PR 60261.
[Val <valentin.bremond gmail.com>, Yann Ylavic]
Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn.c Sat Oct 29 22:17:11 2016
@@ -241,9 +241,9 @@ apr_status_t h2_conn_pre_close(struct h2
return status;
}
-conn_rec *h2_slave_create(conn_rec *master, int slave_id,
- apr_pool_t *parent, apr_allocator_t *allocator)
+conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent)
{
+ apr_allocator_t *allocator;
apr_pool_t *pool;
conn_rec *c;
void *cfg;
@@ -257,9 +257,7 @@ conn_rec *h2_slave_create(conn_rec *mast
* independant of its parent pool in the sense that it can work in
* another thread.
*/
- if (!allocator) {
- apr_allocator_create(&allocator);
- }
+ apr_allocator_create(&allocator);
apr_pool_create_ex(&pool, parent, NULL, allocator);
apr_pool_tag(pool, "h2_slave_conn");
apr_allocator_owner_set(allocator, pool);
@@ -311,21 +309,11 @@ conn_rec *h2_slave_create(conn_rec *mast
return c;
}
-void h2_slave_destroy(conn_rec *slave, apr_allocator_t **pallocator)
+void h2_slave_destroy(conn_rec *slave)
{
- apr_pool_t *parent;
- apr_allocator_t *allocator = apr_pool_allocator_get(slave->pool);
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, slave,
"h2_slave_conn(%ld): destroy (task=%s)", slave->id,
apr_table_get(slave->notes, H2_TASK_ID_NOTE));
- /* Attache the allocator to the parent pool and return it for
- * reuse, otherwise the own is still the slave pool and it will
- * get destroyed with it. */
- parent = apr_pool_parent_get(slave->pool);
- if (pallocator && parent) {
- apr_allocator_owner_set(allocator, parent);
- *pallocator = allocator;
- }
apr_pool_destroy(slave->pool);
}
Modified: httpd/httpd/trunk/modules/http2/h2_conn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.h?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn.h Sat Oct 29 22:17:11 2016
@@ -66,9 +66,8 @@ typedef enum {
h2_mpm_type_t h2_conn_mpm_type(void);
-conn_rec *h2_slave_create(conn_rec *master, int slave_id,
- apr_pool_t *parent, apr_allocator_t *allocator);
-void h2_slave_destroy(conn_rec *slave, apr_allocator_t **pallocator);
+conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent);
+void h2_slave_destroy(conn_rec *slave);
apr_status_t h2_slave_run_pre_connection(conn_rec *slave, apr_socket_t *csd);
void h2_slave_run_connection(conn_rec *slave);
Modified: httpd/httpd/trunk/modules/http2/h2_mplx.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_mplx.c?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_mplx.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_mplx.c Sat Oct 29 22:17:11 2016
@@ -226,11 +226,17 @@ static void purge_streams(h2_mplx *m)
static void h2_mplx_destroy(h2_mplx *m)
{
+ conn_rec **pslave;
ap_assert(m);
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c,
"h2_mplx(%ld): destroy, tasks=%d",
m->id, (int)h2_ihash_count(m->tasks));
check_tx_free(m);
+
+ while (m->spare_slaves->nelts > 0) {
+ pslave = (conn_rec **)apr_array_pop(m->spare_slaves);
+ h2_slave_destroy(*pslave);
+ }
if (m->pool) {
apr_pool_destroy(m->pool);
}
@@ -297,6 +303,7 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr
m->q = h2_iq_create(m->pool, m->max_streams);
m->sready = h2_ihash_create(m->pool, offsetof(h2_stream,id));
m->tasks = h2_ihash_create(m->pool, offsetof(h2_task,stream_id));
+ m->redo_tasks = h2_ihash_create(m->pool, offsetof(h2_task, stream_id));
m->stream_timeout = stream_timeout;
m->workers = workers;
@@ -363,24 +370,20 @@ static void task_destroy(h2_mplx *m, h2_
}
}
- if (task->output.beam) {
- h2_beam_on_produced(task->output.beam, NULL, NULL);
- ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c,
- APLOGNO(03385) "h2_task(%s): destroy "
- "output beam empty=%d, holds proxies=%d",
- task->id,
- h2_beam_empty(task->output.beam),
- h2_beam_holds_proxies(task->output.beam));
- }
+ h2_beam_on_produced(task->output.beam, NULL, NULL);
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c,
+ APLOGNO(03385) "h2_task(%s): destroy "
+ "output beam empty=%d, holds proxies=%d",
+ task->id,
+ h2_beam_empty(task->output.beam),
+ h2_beam_holds_proxies(task->output.beam));
slave = task->c;
reuse_slave = ((m->spare_slaves->nelts < m->spare_slaves->nalloc)
&& !task->rst_error);
h2_ihash_remove(m->tasks, task->stream_id);
- if (m->redo_tasks) {
- h2_ihash_remove(m->redo_tasks, task->stream_id);
- }
+ h2_ihash_remove(m->redo_tasks, task->stream_id);
h2_task_destroy(task);
if (slave) {
@@ -389,7 +392,7 @@ static void task_destroy(h2_mplx *m, h2_
}
else {
slave->sbh = NULL;
- h2_slave_destroy(slave, NULL);
+ h2_slave_destroy(slave);
}
}
@@ -434,18 +437,15 @@ static void stream_done(h2_mplx *m, h2_s
h2_iq_remove(m->q, stream->id);
h2_ihash_remove(m->sready, stream->id);
h2_ihash_remove(m->streams, stream->id);
- if (stream->input) {
- m->tx_handles_reserved += h2_beam_get_files_beamed(stream->input);
- h2_beam_on_consumed(stream->input, NULL, NULL);
- /* Let anyone blocked reading know that there is no more to come */
- h2_beam_abort(stream->input);
- /* Remove mutex after, so that abort still finds cond to signal */
- h2_beam_mutex_set(stream->input, NULL, NULL, NULL);
- }
- if (stream->output) {
- m->tx_handles_reserved += h2_beam_get_files_beamed(stream->output);
- }
+
h2_stream_cleanup(stream);
+ m->tx_handles_reserved += h2_beam_get_files_beamed(stream->input);
+ h2_beam_on_consumed(stream->input, NULL, NULL);
+ /* Let anyone blocked reading know that there is no more to come */
+ h2_beam_abort(stream->input);
+ /* Remove mutex after, so that abort still finds cond to signal */
+ h2_beam_mutex_set(stream->input, NULL, NULL, NULL);
+ m->tx_handles_reserved += h2_beam_get_files_beamed(stream->output);
task = h2_ihash_get(m->tasks, stream->id);
if (task) {
@@ -459,7 +459,7 @@ static void stream_done(h2_mplx *m, h2_s
}
else {
/* already finished */
- task_destroy(m, task, 0);
+ task_destroy(m, task, 1);
}
}
h2_stream_destroy(stream);
@@ -532,12 +532,8 @@ static int task_abort_connection(void *c
if (task->c) {
task->c->aborted = 1;
}
- if (task->input.beam) {
- h2_beam_abort(task->input.beam);
- }
- if (task->output.beam) {
- h2_beam_abort(task->output.beam);
- }
+ h2_beam_abort(task->input.beam);
+ h2_beam_abort(task->output.beam);
}
return 1;
}
@@ -772,11 +768,9 @@ static apr_status_t out_close(h2_mplx *m
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, m->c,
"h2_mplx(%s): close", task->id);
- if (task->output.beam) {
- status = h2_beam_close(task->output.beam);
- h2_beam_log(task->output.beam, task->stream_id, "out_close", m->c,
- APLOG_TRACE2);
- }
+ status = h2_beam_close(task->output.beam);
+ h2_beam_log(task->output.beam, task->stream_id, "out_close", m->c,
+ APLOG_TRACE2);
output_consumed_signal(m, task);
have_out_data_for(m, stream, 0);
return status;
@@ -897,7 +891,7 @@ static h2_task *next_stream_task(h2_mplx
slave = *pslave;
}
else {
- slave = h2_slave_create(m->c, stream->id, m->pool, NULL);
+ slave = h2_slave_create(m->c, stream->id, m->pool);
new_conn = 1;
}
@@ -919,16 +913,13 @@ static h2_task *next_stream_task(h2_mplx
m->max_stream_started = sid;
}
- if (stream->input) {
- h2_beam_timeout_set(stream->input, m->stream_timeout);
- h2_beam_on_consumed(stream->input, stream_input_consumed, m);
- h2_beam_on_file_beam(stream->input, can_beam_file, m);
- h2_beam_mutex_set(stream->input, beam_enter, task->cond, m);
- }
- if (stream->output) {
- h2_beam_buffer_size_set(stream->output, m->stream_max_mem);
- h2_beam_timeout_set(stream->output, m->stream_timeout);
- }
+ h2_beam_timeout_set(stream->input, m->stream_timeout);
+ h2_beam_on_consumed(stream->input, stream_input_consumed, m);
+ h2_beam_on_file_beam(stream->input, can_beam_file, m);
+ h2_beam_mutex_set(stream->input, beam_enter, task->cond, m);
+
+ h2_beam_buffer_size_set(stream->output, m->stream_max_mem);
+ h2_beam_timeout_set(stream->output, m->stream_timeout);
++m->workers_busy;
}
}
@@ -977,10 +968,8 @@ static void task_done(h2_mplx *m, h2_tas
if (ngn) {
apr_off_t bytes = 0;
- if (task->output.beam) {
- h2_beam_send(task->output.beam, NULL, APR_NONBLOCK_READ);
- bytes += h2_beam_get_buffered(task->output.beam);
- }
+ h2_beam_send(task->output.beam, NULL, APR_NONBLOCK_READ);
+ bytes += h2_beam_get_buffered(task->output.beam);
if (bytes > 0) {
/* we need to report consumed and current buffered output
* to the engine. The request will be streamed out or cancelled,
@@ -1001,7 +990,7 @@ static void task_done(h2_mplx *m, h2_tas
}
stream = h2_ihash_get(m->streams, task->stream_id);
- if (!m->aborted && stream && m->redo_tasks
+ if (!m->aborted && stream
&& h2_ihash_get(m->redo_tasks, task->stream_id)) {
/* reset and schedule again */
h2_task_redo(task);
@@ -1152,9 +1141,6 @@ static apr_status_t unschedule_slow_task
h2_task *task;
int n;
- if (!m->redo_tasks) {
- m->redo_tasks = h2_ihash_create(m->pool, offsetof(h2_task, stream_id));
- }
/* Try to get rid of streams that occupy workers. Look for safe requests
* that are repeatable. If none found, fail the connection.
*/
Modified: httpd/httpd/trunk/modules/http2/h2_ngn_shed.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_ngn_shed.c?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_ngn_shed.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_ngn_shed.c Sat Oct 29 22:17:11 2016
@@ -72,7 +72,7 @@ struct h2_req_engine {
const char *type; /* name of the engine type */
apr_pool_t *pool; /* pool for engine specific allocations */
conn_rec *c; /* connection this engine is assigned to */
- h2_task *task; /* the task this engine is base on, running in */
+ h2_task *task; /* the task this engine is based on, running in */
h2_ngn_shed *shed;
unsigned int shutdown : 1; /* engine is being shut down */
Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Sat Oct 29 22:17:11 2016
@@ -82,7 +82,6 @@ apr_status_t h2_session_stream_done(h2_s
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c,
"h2_stream(%ld-%d): EOS bucket cleanup -> done",
session->id, stream->id);
- h2_ihash_remove(session->streams, stream->id);
h2_mplx_stream_done(session->mplx, stream);
dispatch_event(session, H2_SESSION_EV_STREAM_DONE, 0, NULL);
@@ -94,10 +93,9 @@ typedef struct stream_sel_ctx {
h2_stream *candidate;
} stream_sel_ctx;
-static int find_cleanup_stream(void *udata, void *sdata)
+static int find_cleanup_stream(h2_stream *stream, void *ictx)
{
- stream_sel_ctx *ctx = udata;
- h2_stream *stream = sdata;
+ stream_sel_ctx *ctx = ictx;
if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
if (!ctx->session->local.accepting
&& stream->id > ctx->session->local.accepted_max) {
@@ -121,7 +119,7 @@ static void cleanup_streams(h2_session *
ctx.session = session;
ctx.candidate = NULL;
while (1) {
- h2_ihash_iter(session->streams, find_cleanup_stream, &ctx);
+ h2_mplx_stream_do(session->mplx, find_cleanup_stream, &ctx);
if (ctx.candidate) {
h2_session_stream_done(session, ctx.candidate);
ctx.candidate = NULL;
@@ -144,7 +142,6 @@ h2_stream *h2_session_open_stream(h2_ses
stream = h2_stream_open(stream_id, stream_pool, session,
initiated_on);
nghttp2_session_set_stream_user_data(session->ngh2, stream_id, stream);
- h2_ihash_add(session->streams, stream);
if (req) {
h2_stream_set_request(stream, req);
@@ -713,7 +710,6 @@ static void h2_session_destroy(h2_sessio
{
ap_assert(session);
- h2_ihash_clear(session->streams);
if (session->mplx) {
h2_mplx_set_consumed_cb(session->mplx, NULL, NULL);
h2_mplx_release_and_join(session->mplx, session->iowait);
@@ -927,8 +923,6 @@ static h2_session *h2_session_create_int
return NULL;
}
- session->streams = h2_ihash_create(session->pool,
- offsetof(h2_stream, id));
session->mplx = h2_mplx_create(c, session->pool, session->config,
session->s->timeout, workers);
Modified: httpd/httpd/trunk/modules/http2/h2_session.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.h?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.h Sat Oct 29 22:17:11 2016
@@ -86,7 +86,6 @@ typedef struct h2_session {
struct h2_workers *workers; /* for executing stream tasks */
struct h2_filter_cin *cin; /* connection input filter context */
h2_conn_io io; /* io on httpd conn filters */
- struct h2_ihash_t *streams; /* streams handled by this session */
struct nghttp2_session *ngh2; /* the nghttp2 session (internal use) */
h2_session_state state; /* state session is in */
Modified: httpd/httpd/trunk/modules/http2/h2_task.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_task.c?rev=1767128&r1=1767127&r2=1767128&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_task.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_task.c Sat Oct 29 22:17:11 2016
@@ -403,7 +403,7 @@ static apr_status_t h2_filter_parse_h1(a
******************************************************************************/
int h2_task_can_redo(h2_task *task) {
- if (task->input.beam && h2_beam_was_received(task->input.beam)) {
+ if (h2_beam_was_received(task->input.beam)) {
/* cannot repeat that. */
return 0;
}
@@ -420,10 +420,8 @@ void h2_task_redo(h2_task *task)
void h2_task_rst(h2_task *task, int error)
{
task->rst_error = error;
- if (task->input.beam) {
- h2_beam_abort(task->input.beam);
- }
- if (!task->worker_done && task->output.beam) {
+ h2_beam_abort(task->input.beam);
+ if (!task->worker_done) {
h2_beam_abort(task->output.beam);
}
if (task->c) {