Browse Source

H2O: Optimize the implementation (#2260)

Anton Kirilov 9 years ago
parent
commit
5747d630ea

+ 0 - 1
frameworks/C/h2o/CMakeLists.txt

@@ -8,7 +8,6 @@ find_path(MUSTACHE_C_INCLUDE mustache.h)
 find_path(YAJL_INCLUDE yajl/yajl_gen.h)
 set(COMMON_OPTIONS -flto -pthread)
 add_compile_options(-std=gnu11 -pedantic -Wall -Wextra ${COMMON_OPTIONS})
-set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
 set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fstack-protector-all -D_FORTIFY_SOURCE=2")
 set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -Ofast")
 set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} -Ofast")

+ 1 - 1
frameworks/C/h2o/setup.sh

@@ -25,7 +25,7 @@ run_curl()
 
 run_h2o_app()
 {
-	"$1/h2o_app" -a2 -f "$2/template/fortunes.mustache" -m2 "$3" "$4" \
+	"$1/h2o_app" -a2 -f "$2/template/fortunes.mustache" -m8 "$3" "$4" \
 		-d "host=$DBHOST dbname=hello_world user=benchmarkdbuser password=benchmarkdbpass" &
 }
 

+ 37 - 61
frameworks/C/h2o/src/fortune.c

@@ -49,6 +49,7 @@ typedef struct {
 	iovec_list_t *iovec_list_iter;
 	h2o_req_t *req;
 	list_t *result;
+	size_t content_length;
 	size_t num_result;
 	db_query_param_t param;
 	h2o_generator_t generator;
@@ -56,9 +57,8 @@ typedef struct {
 
 static uintmax_t add_iovec(mustache_api_t *api,
                            void *userdata,
-                           char *buffer,
+                           const char *buffer,
                            uintmax_t buffer_size);
-static void cleanup_fortunes(struct st_h2o_generator_t *self, h2o_req_t *req);
 static int compare_fortunes(const list_t *x, const list_t *y);
 static void complete_fortunes(struct st_h2o_generator_t *self, h2o_req_t *req);
 static list_t *get_sorted_sublist(list_t *head);
@@ -76,7 +76,7 @@ static list_t *sort_fortunes(list_t *head);
 
 static uintmax_t add_iovec(mustache_api_t *api,
                            void *userdata,
-                           char *buffer,
+                           const char *buffer,
                            uintmax_t buffer_size)
 {
 	IGNORE_FUNCTION_PARAMETER(api);
@@ -101,33 +101,14 @@ static uintmax_t add_iovec(mustache_api_t *api,
 	}
 
 	if (ret) {
-		iovec_list->iov[iovec_list->iovcnt].base = buffer;
+		iovec_list->iov[iovec_list->iovcnt].base = (char *) buffer;
 		iovec_list->iov[iovec_list->iovcnt++].len = buffer_size;
+		fortune_ctx->content_length += buffer_size;
 	}
 
 	return ret;
 }
 
-static void cleanup_fortunes(struct st_h2o_generator_t *self, h2o_req_t *req)
-{
-	IGNORE_FUNCTION_PARAMETER(req);
-
-	fortune_ctx_t * const fortune_ctx = H2O_STRUCT_FROM_MEMBER(fortune_ctx_t,
-	                                                           generator,
-	                                                           self);
-	const list_t *iter = fortune_ctx->result;
-
-	if (iter)
-		do {
-			const fortune_t * const fortune = H2O_STRUCT_FROM_MEMBER(fortune_t, l, iter);
-
-			if (fortune->data)
-				PQclear(fortune->data);
-
-			iter = iter->next;
-		} while (iter);
-}
-
 static int compare_fortunes(const list_t *x, const list_t *y)
 {
 	const fortune_t * const f1 = H2O_STRUCT_FROM_MEMBER(fortune_t, l, x);
@@ -146,19 +127,12 @@ static void complete_fortunes(struct st_h2o_generator_t *self, h2o_req_t *req)
 	fortune_ctx_t * const fortune_ctx = H2O_STRUCT_FROM_MEMBER(fortune_ctx_t,
 	                                                           generator,
 	                                                           self);
+	iovec_list_t * const iovec_list = H2O_STRUCT_FROM_MEMBER(iovec_list_t,
+	                                                         l,
+	                                                         fortune_ctx->iovec_list);
 
-	if (fortune_ctx->iovec_list) {
-		iovec_list_t * const iovec_list = H2O_STRUCT_FROM_MEMBER(iovec_list_t,
-		                                                         l,
-		                                                         fortune_ctx->iovec_list);
-
-		fortune_ctx->iovec_list = iovec_list->l.next;
-		h2o_send(fortune_ctx->req, iovec_list->iov, iovec_list->iovcnt, false);
-	}
-	else {
-		h2o_send(req, NULL, 0, true);
-		cleanup_fortunes(self, req);
-	}
+	fortune_ctx->iovec_list = iovec_list->l.next;
+	h2o_send(req, iovec_list->iov, iovec_list->iovcnt, !fortune_ctx->iovec_list);
 }
 
 static list_t *get_sorted_sublist(list_t *head)
@@ -218,7 +192,6 @@ static void on_fortune_error(db_query_param_t *param, const char *error_string)
 	                                                           param,
 	                                                           param);
 
-	cleanup_fortunes(&fortune_ctx->generator, fortune_ctx->req);
 	send_error(BAD_GATEWAY, error_string, fortune_ctx->req);
 }
 
@@ -236,37 +209,45 @@ static result_return_t on_fortune_result(db_query_param_t *param, PGresult *resu
 		ret = SUCCESS;
 
 		for (size_t i = 0; i < num_rows; i++) {
+			const char * const message_data = PQgetvalue(result, i, 1);
+			h2o_iovec_t message = h2o_htmlescape(&fortune_ctx->req->pool,
+			                                     message_data,
+			                                     PQgetlength(result, i, 1));
+			const size_t id_len = PQgetlength(result, i, 0);
+			const size_t fortune_size = offsetof(fortune_t, data) + id_len +
+			                            (message_data == message.base ? message.len : 0);
 			fortune_t * const fortune = h2o_mem_alloc_pool(&fortune_ctx->req->pool,
-			                                               sizeof(*fortune));
+			                                               fortune_size);
 
 			if (fortune) {
-				memset(fortune, 0, sizeof(*fortune));
-				fortune->id.base = PQgetvalue(result, i, 0);
-				fortune->id.len = PQgetlength(result, i, 0);
-				fortune->message = h2o_htmlescape(&fortune_ctx->req->pool,
-				                                  PQgetvalue(result, i, 1),
-				                                  PQgetlength(result, i, 1));
+				memset(fortune, 0, offsetof(fortune_t, data));
+				memcpy(fortune->data, PQgetvalue(result, i, 0), id_len);
+				fortune->id.base = fortune->data;
+				fortune->id.len = id_len;
+
+				if (message_data == message.base) {
+					message.base = fortune->data + id_len;
+					memcpy(message.base, message_data, message.len);
+				}
+
+				fortune->message = message;
 				fortune->l.next = fortune_ctx->result;
 				fortune_ctx->result = &fortune->l;
 				fortune_ctx->num_result++;
-
-				if (!i)
-					fortune->data = result;
 			}
 			else {
-				cleanup_fortunes(&fortune_ctx->generator, fortune_ctx->req);
 				send_error(INTERNAL_SERVER_ERROR, MEM_ALLOC_ERR_MSG, fortune_ctx->req);
 				ret = DONE;
-
-				if (!i)
-					PQclear(result);
-
 				break;
 			}
 		}
+
+		PQclear(result);
 	}
-	else if (result)
+	else if (result) {
 		PQclear(result);
+		send_error(BAD_GATEWAY, PQresultErrorMessage(result), fortune_ctx->req);
+	}
 	else {
 		mustache_api_t api = {.sectget = on_fortune_section,
 		                      .varget = on_fortune_variable,
@@ -286,18 +267,15 @@ static result_return_t on_fortune_result(db_query_param_t *param, PGresult *resu
 
 		if (mustache_render(&api, fortune_ctx, ctx->global_data->fortunes_template)) {
 			fortune_ctx->iovec_list = iovec_list->l.next;
-			set_default_response_param(HTML, fortune_ctx->req);
+			set_default_response_param(HTML, fortune_ctx->content_length, fortune_ctx->req);
 			h2o_start_response(fortune_ctx->req, &fortune_ctx->generator);
 			h2o_send(fortune_ctx->req,
 			         iovec_list->iov,
 			         iovec_list->iovcnt,
-			         false);
+			         !fortune_ctx->iovec_list);
 		}
-		else {
-			cleanup_fortunes(&fortune_ctx->generator, fortune_ctx->req);
+		else
 			send_error(INTERNAL_SERVER_ERROR, MEM_ALLOC_ERR_MSG, fortune_ctx->req);
-			ret = DONE;
-		}
 	}
 
 	return ret;
@@ -335,7 +313,6 @@ static void on_fortune_timeout(db_query_param_t *param)
 	                                                           param,
 	                                                           param);
 
-	cleanup_fortunes(&fortune_ctx->generator, fortune_ctx->req);
 	send_error(GATEWAY_TIMEOUT, DB_TIMEOUT_ERROR, fortune_ctx->req);
 }
 
@@ -401,7 +378,6 @@ int fortunes(struct st_h2o_handler_t *self, h2o_req_t *req)
 			fortune->message.len = sizeof(NEW_FORTUNE_MESSAGE) - 1;
 			memset(fortune_ctx, 0, sizeof(*fortune_ctx));
 			fortune_ctx->generator.proceed = complete_fortunes;
-			fortune_ctx->generator.stop = cleanup_fortunes;
 			fortune_ctx->num_result = 1;
 			fortune_ctx->param.command = FORTUNE_TABLE_NAME;
 			fortune_ctx->param.on_error = on_fortune_error;

+ 1 - 1
frameworks/C/h2o/src/fortune.h

@@ -28,9 +28,9 @@
 
 typedef struct {
 	list_t l;
-	PGresult *data;
 	h2o_iovec_t id;
 	h2o_iovec_t message;
+	char data[];
 } fortune_t;
 
 int fortunes(struct st_h2o_handler_t *self, h2o_req_t *req);

+ 35 - 51
frameworks/C/h2o/src/request_handler.c

@@ -20,6 +20,7 @@
 #include <assert.h>
 #include <h2o.h>
 #include <stdalign.h>
+#include <stdint.h>
 #include <string.h>
 #include <yajl/yajl_gen.h>
 
@@ -29,56 +30,29 @@
 #include "utility.h"
 #include "world.h"
 
-typedef struct {
-	yajl_gen gen;
-	h2o_generator_t h2o_generator;
-} json_ctx_t;
+#define HELLO_RESPONSE "Hello, World!"
 
-static void cleanup_json_response(struct st_h2o_generator_t *self, h2o_req_t *req);
-static void complete_json_response(struct st_h2o_generator_t *self, h2o_req_t *req);
 static int json_serializer(struct st_h2o_handler_t *self, h2o_req_t *req);
 static int plaintext(struct st_h2o_handler_t *self, h2o_req_t *req);
 static const char *status_code_to_string(http_status_code_t status_code);
 
-static void cleanup_json_response(struct st_h2o_generator_t *self, h2o_req_t *req)
-{
-	IGNORE_FUNCTION_PARAMETER(req);
-
-	json_ctx_t * const json_ctx = H2O_STRUCT_FROM_MEMBER(json_ctx_t, h2o_generator, self);
-
-	yajl_gen_free(json_ctx->gen);
-}
-
-static void complete_json_response(struct st_h2o_generator_t *self, h2o_req_t *req)
-{
-	h2o_send(req, NULL, 0, true);
-	cleanup_json_response(self, req);
-}
-
 static int json_serializer(struct st_h2o_handler_t *self, h2o_req_t *req)
 {
 	IGNORE_FUNCTION_PARAMETER(self);
 
-	json_ctx_t * const json_ctx = h2o_mem_alloc_pool(&req->pool, sizeof(*json_ctx));
-
-	if (json_ctx) {
-		memset(json_ctx, 0, sizeof(*json_ctx));
-		json_ctx->gen = get_json_generator(&req->pool);
-		json_ctx->h2o_generator.proceed = complete_json_response;
-		json_ctx->h2o_generator.stop = cleanup_json_response;
+	const yajl_gen gen = get_json_generator(&req->pool);
 
-		if (json_ctx->gen) {
-			CHECK_YAJL_STATUS(yajl_gen_map_open, json_ctx->gen);
-			CHECK_YAJL_STATUS(yajl_gen_string, json_ctx->gen, YAJL_STRLIT("message"));
-			CHECK_YAJL_STATUS(yajl_gen_string, json_ctx->gen, YAJL_STRLIT("Hello, World!"));
-			CHECK_YAJL_STATUS(yajl_gen_map_close, json_ctx->gen);
+	if (gen) {
+		CHECK_YAJL_STATUS(yajl_gen_map_open, gen);
+		CHECK_YAJL_STATUS(yajl_gen_string, gen, YAJL_STRLIT("message"));
+		CHECK_YAJL_STATUS(yajl_gen_string, gen, YAJL_STRLIT(HELLO_RESPONSE));
+		CHECK_YAJL_STATUS(yajl_gen_map_close, gen);
 
-			if (!send_json_response(json_ctx->gen, &json_ctx->h2o_generator, req))
-				return 0;
+		if (!send_json_response(gen, NULL, req))
+			return 0;
 
 error_yajl:
-			yajl_gen_free(json_ctx->gen);
-		}
+		yajl_gen_free(gen);
 	}
 
 	send_error(INTERNAL_SERVER_ERROR, MEM_ALLOC_ERR_MSG, req);
@@ -88,8 +62,8 @@ error_yajl:
 static int plaintext(struct st_h2o_handler_t *self, h2o_req_t *req)
 {
 	IGNORE_FUNCTION_PARAMETER(self);
-	set_default_response_param(PLAIN, req);
-	h2o_send_inline(req, H2O_STRLIT("Hello, World!"));
+	set_default_response_param(PLAIN, sizeof(HELLO_RESPONSE) - 1, req);
+	h2o_send_inline(req, H2O_STRLIT(HELLO_RESPONSE));
 	return 0;
 }
 
@@ -197,20 +171,29 @@ void send_error(http_status_code_t status_code, const char *body, h2o_req_t *req
 
 int send_json_response(yajl_gen gen, h2o_generator_t *h2o_generator, h2o_req_t *req)
 {
+	const unsigned char *buf;
 	h2o_iovec_t h2o_iovec = {.len = 0};
 	int ret = EXIT_FAILURE;
 
-	static_assert(sizeof(h2o_iovec.base) == sizeof(const unsigned char *) &&
-	              alignof(h2o_iovec.base) == alignof(const unsigned char *),
-	              "Types must be compatible.");
-
-	if (yajl_gen_get_buf(gen,
-	                     (const unsigned char **) &h2o_iovec.base,
-	                     &h2o_iovec.len) == yajl_gen_status_ok) {
-		set_default_response_param(JSON, req);
-		h2o_start_response(req, h2o_generator);
-		h2o_send(req, &h2o_iovec, 1, false);
-		ret = EXIT_SUCCESS;
+	if (yajl_gen_get_buf(gen, &buf, &h2o_iovec.len) == yajl_gen_status_ok) {
+		if (h2o_generator) {
+			h2o_iovec.base = (char *) buf;
+			set_default_response_param(JSON, SIZE_MAX, req);
+			h2o_start_response(req, h2o_generator);
+			h2o_send(req, &h2o_iovec, 1, false);
+			ret = EXIT_SUCCESS;
+		}
+		else {
+			h2o_iovec.base = h2o_mem_alloc_pool(&req->pool, h2o_iovec.len);
+
+			if (h2o_iovec.base) {
+				memcpy(h2o_iovec.base, buf, h2o_iovec.len);
+				yajl_gen_free(gen);
+				set_default_response_param(JSON, h2o_iovec.len, req);
+				h2o_send_inline(req, h2o_iovec.base, h2o_iovec.len);
+				ret = EXIT_SUCCESS;
+			}
+		}
 	}
 
 	return ret;
@@ -228,8 +211,9 @@ void send_service_unavailable_error(const char *body, h2o_req_t *req)
 	                   H2O_SEND_ERROR_KEEP_HEADERS);
 }
 
-void set_default_response_param(content_type_t content_type, h2o_req_t *req)
+void set_default_response_param(content_type_t content_type, size_t content_length, h2o_req_t *req)
 {
+	req->res.content_length = content_length;
 	req->res.status = OK;
 	req->res.reason = status_code_to_string(req->res.status);
 

+ 3 - 1
frameworks/C/h2o/src/request_handler.h

@@ -46,6 +46,8 @@ void register_request_handlers(h2o_hostconf_t *hostconf, h2o_access_log_filehand
 void send_error(http_status_code_t status_code, const char *body, h2o_req_t *req);
 int send_json_response(yajl_gen gen, h2o_generator_t *h2o_generator, h2o_req_t *req);
 void send_service_unavailable_error(const char *body, h2o_req_t *req);
-void set_default_response_param(content_type_t content_type, h2o_req_t *req);
+void set_default_response_param(content_type_t content_type,
+                                size_t content_length,
+                                h2o_req_t *req);
 
 #endif // REQUEST_H_

+ 8 - 2
frameworks/C/h2o/src/template.c

@@ -46,7 +46,10 @@ static uintmax_t read_template(mustache_api_t *api,
                                void *userdata,
                                char *buffer,
                                uintmax_t buffer_size);
-static void template_error(mustache_api_t *api, void *userdata, uintmax_t lineno, char *error);
+static void template_error(mustache_api_t *api,
+                           void *userdata,
+                           uintmax_t lineno,
+                           const char *error);
 
 static uintmax_t prerender_section(mustache_api_t *api,
                                    void *userdata,
@@ -101,7 +104,10 @@ static uintmax_t read_template(mustache_api_t *api,
 	return fread(buffer, sizeof(*buffer), buffer_size, template_input->input);
 }
 
-static void template_error(mustache_api_t *api, void *userdata, uintmax_t lineno, char *error)
+static void template_error(mustache_api_t *api,
+                           void *userdata,
+                           uintmax_t lineno,
+                           const char *error)
 {
 	IGNORE_FUNCTION_PARAMETER(api);
 

+ 5 - 28
frameworks/C/h2o/src/world.c

@@ -54,7 +54,6 @@ typedef struct {
 typedef struct {
 	db_query_param_t param;
 	yajl_gen gen;
-	h2o_generator_t h2o_generator;
 	const char *id_pointer;
 	h2o_req_t *req;
 	uint32_t id;
@@ -70,8 +69,6 @@ typedef struct {
 	query_result_t res[];
 } multiple_query_ctx_t;
 
-static void complete_request(struct st_h2o_generator_t *self, h2o_req_t *req);
-static void cleanup_request(struct st_h2o_generator_t *self, h2o_req_t *req);
 static int do_multiple_queries(update_state_t update_state, h2o_req_t *req);
 static int initialize_single_query_context(h2o_req_t *req,
                                            on_result_t on_result,
@@ -85,27 +82,9 @@ static int on_update_write_ready(db_query_param_t *param, PGconn *db_conn);
 static int serialize_item(uint32_t id, uint32_t random_number, yajl_gen gen);
 static void serialize_items(const query_result_t *res,
                             size_t num_result,
-                            h2o_generator_t *h2o_generator,
                             yajl_gen gen,
                             h2o_req_t *req);
 
-static void cleanup_request(struct st_h2o_generator_t *self, h2o_req_t *req)
-{
-	IGNORE_FUNCTION_PARAMETER(req);
-
-	single_query_ctx_t * const query_ctx = H2O_STRUCT_FROM_MEMBER(single_query_ctx_t,
-	                                                              h2o_generator,
-	                                                              self);
-
-	yajl_gen_free(query_ctx->gen);
-}
-
-static void complete_request(struct st_h2o_generator_t *self, h2o_req_t *req)
-{
-	h2o_send(req, NULL, 0, true);
-	cleanup_request(self, req);
-}
-
 static int do_multiple_queries(update_state_t update_state, h2o_req_t *req)
 {
 	thread_context_t * const ctx = H2O_STRUCT_FROM_MEMBER(thread_context_t,
@@ -178,8 +157,6 @@ static int initialize_single_query_context(h2o_req_t *req,
 	query_ctx->gen = get_json_generator(&req->pool);
 
 	if (query_ctx->gen) {
-		query_ctx->h2o_generator.proceed = complete_request;
-		query_ctx->h2o_generator.stop = cleanup_request;
 		query_ctx->id_format = 1;
 		query_ctx->id_len = sizeof(query_ctx->id);
 		query_ctx->id_pointer = (const char *) &query_ctx->id;
@@ -251,7 +228,6 @@ static result_return_t on_multiple_query_result(db_query_param_t *param, PGresul
 		else if (query_ctx->update_state == NO_UPDATE) {
 			serialize_items(query_ctx->res,
 			                query_ctx->num_result,
-			                &query_ctx->single.h2o_generator,
 			                query_ctx->single.gen,
 			                query_ctx->single.req);
 			return DONE;
@@ -300,7 +276,7 @@ static result_return_t on_single_query_result(db_query_param_t *param, PGresult
 		PQclear(result);
 
 		if (!serialize_item(ntohl(query_ctx->id), random_number, query_ctx->gen) &&
-		    !send_json_response(query_ctx->gen, &query_ctx->h2o_generator, query_ctx->req))
+		    !send_json_response(query_ctx->gen, NULL, query_ctx->req))
 			return DONE;
 
 		send_error(INTERNAL_SERVER_ERROR, MEM_ALLOC_ERR_MSG, query_ctx->req);
@@ -342,7 +318,6 @@ static result_return_t on_update_result(db_query_param_t *param, PGresult *resul
 
 			serialize_items(query_ctx->res,
 			                query_ctx->num_result,
-			                &query_ctx->single.h2o_generator,
 			                query_ctx->single.gen,
 			                query_ctx->single.req);
 			ret = DONE;
@@ -354,6 +329,7 @@ static result_return_t on_update_result(db_query_param_t *param, PGresult *resul
 	PQclear(result);
 	return ret;
 error:
+	yajl_gen_free(query_ctx->single.gen);
 	send_error(BAD_GATEWAY, PQresultErrorMessage(result), query_ctx->single.req);
 	PQclear(result);
 	return DONE;
@@ -440,7 +416,6 @@ error_yajl:
 
 static void serialize_items(const query_result_t *res,
                             size_t num_result,
-                            h2o_generator_t *h2o_generator,
                             yajl_gen gen,
                             h2o_req_t *req)
 {
@@ -452,9 +427,11 @@ static void serialize_items(const query_result_t *res,
 
 	CHECK_YAJL_STATUS(yajl_gen_array_close, gen);
 
-	if (send_json_response(gen, h2o_generator, req))
+	if (send_json_response(gen, NULL, req)) {
 error_yajl:
+		yajl_gen_free(gen);
 		send_error(INTERNAL_SERVER_ERROR, MEM_ALLOC_ERR_MSG, req);
+	}
 }
 
 int multiple_queries(struct st_h2o_handler_t *self, h2o_req_t *req)

+ 2 - 2
toolset/setup/linux/systools/mustache-c.sh

@@ -10,8 +10,8 @@ BUILD_DIR="${MUSTACHE_C_HOME}_build"
 
 git clone 'https://github.com/x86-64/mustache-c.git' "$BUILD_DIR"
 pushd "$BUILD_DIR"
-git checkout c00262c9bb57b7871b1c68930e85c8f0e4c8c7d9
-./configure --prefix="$MUSTACHE_C_HOME"
+git checkout 55dafd1e95adaca90ea50efb9a8573786514c85a
+CFLAGS="-march=native" ./configure --prefix="$MUSTACHE_C_HOME"
 make -j "$(nproc)" install
 popd
 rm -rf "$BUILD_DIR"

+ 1 - 1
toolset/setup/linux/webservers/h2o.sh

@@ -14,7 +14,7 @@ pushd "${IROOT}"
 fw_get -O "https://github.com/h2o/h2o/archive/$ARCHIVE"
 fw_untar "$ARCHIVE"
 pushd "$BUILD_DIR"
-cmake -DCMAKE_INSTALL_PREFIX="$H2O_HOME"
+cmake -DCMAKE_INSTALL_PREFIX="$H2O_HOME" -DCMAKE_C_FLAGS="-march=native"
 make -j "$(nproc)" install
 popd
 rm -rf "$BUILD_DIR"