Commit dbb70423608c8f7e09aa1368f69f19b9c9d57be0
1 parent
31b7d755
fixed the non keep-alive performance issue as well as i lower memory usage by us…
…ing a single read and write circular buffer for every connection. @TODO: i noticed a server hang while getting large data (my image) with non keep-alive connections. Additionally an incomplete keep-alive request might stop the server now as the lock on the read buffer will not be released.
Showing
17 changed files
with
158 additions
and
62 deletions
... | ... | @@ -18,11 +18,19 @@ |
18 | 18 | |
19 | 19 | #define ECBUFOVFL 100 |
20 | 20 | |
21 | +#ifndef TRUE | |
22 | +#define TRUE ((void *)1) | |
23 | +#endif | |
24 | + | |
25 | +#ifndef FALSE | |
26 | +#define FALSE ((void *)0) | |
27 | +#endif | |
21 | 28 | |
22 | 29 | CLASS(Cbuf) { |
23 | 30 | char * shm_name; // shared memory identifier |
24 | 31 | |
25 | 32 | char * data; |
33 | + void * lock; | |
26 | 34 | |
27 | 35 | size_t bsize; |
28 | 36 | size_t bused; |
... | ... | @@ -47,6 +55,9 @@ void cbufIncWrite(Cbuf this, size_t n); |
47 | 55 | size_t cbufGetFree(Cbuf this); |
48 | 56 | char cbufIsEmpty(Cbuf this); |
49 | 57 | void cbufSkipNonAlpha(Cbuf this); |
58 | +void * cbufIsLocked(Cbuf this); | |
59 | +void cbufLock(Cbuf this); | |
60 | +void cbufRelease(Cbuf this); | |
50 | 61 | |
51 | 62 | #endif // __RINGBUFFER_H__ |
52 | 63 | ... | ... |
... | ... | @@ -6,20 +6,13 @@ |
6 | 6 | #include "http/message/queue.h" |
7 | 7 | #include "cbuf.h" |
8 | 8 | |
9 | -#define HTTP_REQUEST_PARSER_BUFFER_MAX 8192 | |
9 | +#ifndef TRUE | |
10 | +#define TRUE ((void *)1) | |
11 | +#endif | |
10 | 12 | |
11 | - | |
12 | -/** | |
13 | - * limits to stop invalid requests from killing | |
14 | - * the server. | |
15 | - * If any of these limits is reached the server | |
16 | - * will send an error message and kill the connection | |
17 | - * immediate. | |
18 | - * | |
19 | - * The given limits include any trailing \r\n | |
20 | - */ | |
21 | -#define HTTP_REQUEST_LINE_MAX 8192 | |
22 | -#define HTTP_REQUEST_HEADER_LINE_MAX 2048 | |
13 | +#ifndef FALSE | |
14 | +#define FALSE ((void *)0) | |
15 | +#endif | |
23 | 16 | |
24 | 17 | |
25 | 18 | typedef enum e_HttpRequestState { |
... | ... | @@ -33,6 +26,7 @@ typedef enum e_HttpRequestState { |
33 | 26 | |
34 | 27 | CLASS(HttpRequestParser) { |
35 | 28 | Cbuf buffer; |
29 | + void * ourLock; | |
36 | 30 | |
37 | 31 | HttpMessageQueue request_queue; |
38 | 32 | HttpRequest cur_request; | ... | ... |
... | ... | @@ -8,7 +8,13 @@ |
8 | 8 | #include "http/message/queue.h" |
9 | 9 | #include "cbuf.h" |
10 | 10 | |
11 | -#define RESPONSE_WRITER_MAX_BUF 131072 | |
11 | +#ifndef TRUE | |
12 | +#define TRUE ((void *)1) | |
13 | +#endif | |
14 | + | |
15 | +#ifndef FALSE | |
16 | +#define FALSE ((void *)0) | |
17 | +#endif | |
12 | 18 | |
13 | 19 | |
14 | 20 | typedef enum e_HttpResponseState { |
... | ... | @@ -19,6 +25,7 @@ typedef enum e_HttpResponseState { |
19 | 25 | |
20 | 26 | CLASS(HttpResponseWriter) { |
21 | 27 | Cbuf buffer; |
28 | + void * ourLock; | |
22 | 29 | |
23 | 30 | HttpMessageQueue response_queue; |
24 | 31 | HttpResponse cur_response; | ... | ... |
... | ... | @@ -6,10 +6,25 @@ |
6 | 6 | #include "class.h" |
7 | 7 | #include "http/request/parser.h" |
8 | 8 | #include "http/response/writer.h" |
9 | +#include "cbuf.h" | |
10 | + | |
11 | +#define RESPONSE_WRITER_MAX_BUF 32768 | |
12 | +#define REQUEST_PARSER_BUFFER_MAX 8192 | |
13 | + | |
14 | +#ifndef TRUE | |
15 | +#define TRUE ((void *)1) | |
16 | +#endif | |
17 | + | |
18 | +#ifndef FALSE | |
19 | +#define FALSE ((void *)0) | |
20 | +#endif | |
9 | 21 | |
10 | 22 | CLASS(HttpWorker) { |
11 | 23 | char * id; |
12 | 24 | |
25 | + Cbuf pbuf; | |
26 | + Cbuf wbuf; | |
27 | + | |
13 | 28 | HttpRequestParser parser; |
14 | 29 | HttpResponseWriter writer; |
15 | 30 | }; | ... | ... |
... | ... | @@ -13,7 +13,7 @@ CB = cbuf.c cbuf/read.c cbuf/write.c \ |
13 | 13 | cbuf/get_line.c cbuf/set_data.c cbuf/get_data.c \ |
14 | 14 | cbuf/addr_index.c cbuf/get_free.c cbuf/get_read.c cbuf/get_write.c \ |
15 | 15 | cbuf/inc_read.c cbuf/inc_write.c cbuf/is_empty.c cbuf/memchr.c \ |
16 | - cbuf/skip_non_alpha.c | |
16 | + cbuf/skip_non_alpha.c cbuf/is_locked.c cbuf/lock.c cbuf/release.c | |
17 | 17 | MSG = http/message.c http/message/queue.c http/message/has_keep_alive.c \ |
18 | 18 | http/message/header_size_get.c http/message/header_to_string.c |
19 | 19 | REQ = http/request.c | ... | ... |
src/cbuf/is_locked.c
0 → 100644
src/cbuf/lock.c
0 → 100644
src/cbuf/release.c
0 → 100644
... | ... | @@ -18,12 +18,8 @@ void |
18 | 18 | ctor(void * _this, va_list * params) |
19 | 19 | { |
20 | 20 | HttpRequestParser this = _this; |
21 | - char * id = va_arg(*params, char*); | |
22 | - char cbuf_id[100]; | |
23 | 21 | |
24 | - sprintf(cbuf_id, "%s_%s", "parser", id); | |
25 | - | |
26 | - this->buffer = new(Cbuf, cbuf_id, HTTP_REQUEST_PARSER_BUFFER_MAX); | |
22 | + this->buffer = va_arg(* params, Cbuf); | |
27 | 23 | this->request_queue = new(HttpMessageQueue); |
28 | 24 | } |
29 | 25 | |
... | ... | @@ -34,7 +30,9 @@ dtor(void * _this) |
34 | 30 | HttpRequestParser this = _this; |
35 | 31 | |
36 | 32 | delete(&(this->request_queue)); |
37 | - delete(&(this->buffer)); | |
33 | + | |
34 | + if (TRUE == this->ourLock) | |
35 | + cbufRelease(this->buffer); | |
38 | 36 | |
39 | 37 | if (NULL != this->cur_request) |
40 | 38 | delete(&(this->cur_request)); | ... | ... |
... | ... | @@ -16,6 +16,15 @@ httpRequestParserParse(HttpRequestParser this, int fd) |
16 | 16 | ssize_t read; |
17 | 17 | char * line; |
18 | 18 | |
19 | + if (cbufIsLocked(this->buffer)) { | |
20 | + if (FALSE == this->ourLock) | |
21 | + return 0; | |
22 | + } | |
23 | + else { | |
24 | + cbufLock(this->buffer); | |
25 | + this->ourLock = TRUE; | |
26 | + } | |
27 | + | |
19 | 28 | if(0 > (read = cbufRead(this->buffer, fd))) { |
20 | 29 | return read; |
21 | 30 | } |
... | ... | @@ -28,6 +37,12 @@ httpRequestParserParse(HttpRequestParser this, int fd) |
28 | 37 | this->cur_request = new(HttpRequest); |
29 | 38 | this->state = HTTP_REQUEST_START; |
30 | 39 | } |
40 | + else { | |
41 | + cbufRelease(this->buffer); | |
42 | + this->ourLock = FALSE; | |
43 | + cont = 0; | |
44 | + } | |
45 | + | |
31 | 46 | break; |
32 | 47 | |
33 | 48 | case HTTP_REQUEST_START: |
... | ... | @@ -99,13 +114,6 @@ httpRequestParserParse(HttpRequestParser this, int fd) |
99 | 114 | this->cur_request = NULL; |
100 | 115 | |
101 | 116 | /** |
102 | - * dont continue loop if input buffer is empty | |
103 | - */ | |
104 | - if (cbufIsEmpty(this->buffer)) { | |
105 | - cont = 0; | |
106 | - } | |
107 | - | |
108 | - /** | |
109 | 117 | * prepare for next request |
110 | 118 | */ |
111 | 119 | this->state = HTTP_REQUEST_GARBAGE; | ... | ... |
... | ... | @@ -13,12 +13,8 @@ void |
13 | 13 | ctor(void * _this, va_list * params) |
14 | 14 | { |
15 | 15 | HttpResponseWriter this = _this; |
16 | - char * id = va_arg(*params, char*); | |
17 | - char cbuf_id[100]; | |
18 | 16 | |
19 | - sprintf(cbuf_id, "%s_%s", "writer", id); | |
20 | - | |
21 | - this->buffer = new(Cbuf, cbuf_id, RESPONSE_WRITER_MAX_BUF); | |
17 | + this->buffer = va_arg(*params, Cbuf); | |
22 | 18 | this->response_queue = new(HttpMessageQueue); |
23 | 19 | } |
24 | 20 | |
... | ... | @@ -29,22 +25,15 @@ dtor(void * _this) |
29 | 25 | HttpResponseWriter this = _this; |
30 | 26 | |
31 | 27 | delete(&(this->response_queue)); |
32 | - delete(&(this->buffer)); | |
28 | + | |
29 | + if (TRUE == this->ourLock) | |
30 | + cbufRelease(this->buffer); | |
33 | 31 | |
34 | 32 | if (NULL != this->cur_response) |
35 | 33 | delete(&(this->cur_response)); |
36 | 34 | } |
37 | 35 | |
38 | -static | |
39 | -void | |
40 | -_clone(void * _this, void * _base) | |
41 | -{ | |
42 | - HttpResponseWriter this = _this; | |
43 | - | |
44 | - this->response_queue = new(HttpMessageQueue); | |
45 | -} | |
46 | - | |
47 | -INIT_IFACE(Class, ctor, dtor, _clone); | |
36 | +INIT_IFACE(Class, ctor, dtor, NULL); | |
48 | 37 | INIT_IFACE(StreamWriter, (fptr_streamWriterWrite)httpResponseWriterWrite); |
49 | 38 | CREATE_CLASS(HttpResponseWriter, NULL, IFACE(Class), IFACE(StreamWriter)); |
50 | 39 | ... | ... |
... | ... | @@ -25,6 +25,15 @@ httpResponseWriterWrite(HttpResponseWriter this, int fd) |
25 | 25 | HttpMessage message = (HttpMessage)this->cur_response; |
26 | 26 | int cont = 1; |
27 | 27 | |
28 | + if (cbufIsLocked(this->buffer)) { | |
29 | + if (FALSE == this->ourLock) | |
30 | + return 0; | |
31 | + } | |
32 | + else { | |
33 | + cbufLock(this->buffer); | |
34 | + this->ourLock = TRUE; | |
35 | + } | |
36 | + | |
28 | 37 | while (cont) { |
29 | 38 | switch (this->state) { |
30 | 39 | case HTTP_RESPONSE_GET: |
... | ... | @@ -42,7 +51,9 @@ httpResponseWriterWrite(HttpResponseWriter this, int fd) |
42 | 51 | this->state = HTTP_RESPONSE_WRITE; |
43 | 52 | } |
44 | 53 | else { |
45 | - cont = 0; | |
54 | + cbufRelease(this->buffer); | |
55 | + this->ourLock = FALSE; | |
56 | + cont = 0; | |
46 | 57 | } |
47 | 58 | break; |
48 | 59 | |
... | ... | @@ -113,10 +124,14 @@ httpResponseWriterWrite(HttpResponseWriter this, int fd) |
113 | 124 | * return to the caller with a 0 indicating that the |
114 | 125 | * underlying connection should be closed. |
115 | 126 | */ |
127 | + cbufRelease(this->buffer); | |
128 | + this->ourLock = FALSE; | |
116 | 129 | delete(&this->cur_response); |
117 | 130 | return -1; |
118 | 131 | } |
119 | 132 | |
133 | + cbufRelease(this->buffer); | |
134 | + this->ourLock = FALSE; | |
120 | 135 | delete(&this->cur_response); |
121 | 136 | |
122 | 137 | break; | ... | ... |
... | ... | @@ -2,6 +2,7 @@ |
2 | 2 | #include <stdarg.h> |
3 | 3 | #include <stdlib.h> |
4 | 4 | #include <string.h> |
5 | +#include <stdio.h> | |
5 | 6 | |
6 | 7 | #include "class.h" |
7 | 8 | #include "http/worker.h" |
... | ... | @@ -19,12 +20,19 @@ ctor(void * _this, va_list * params) |
19 | 20 | { |
20 | 21 | HttpWorker this = _this; |
21 | 22 | char * id = va_arg(*params, char *); |
23 | + char cbuf_id[100]; | |
22 | 24 | |
23 | 25 | this->id = malloc(strlen(id) + 1); |
24 | 26 | strcpy(this->id, id); |
25 | 27 | |
26 | - this->parser = new(HttpRequestParser, this->id); | |
27 | - this->writer = new(HttpResponseWriter, this->id); | |
28 | + sprintf(cbuf_id, "%s_%s", "parser", id); | |
29 | + this->pbuf = new(Cbuf, cbuf_id, REQUEST_PARSER_BUFFER_MAX); | |
30 | + | |
31 | + sprintf(cbuf_id, "%s_%s", "writer", id); | |
32 | + this->wbuf = new(Cbuf, cbuf_id, RESPONSE_WRITER_MAX_BUF); | |
33 | + | |
34 | + this->parser = new(HttpRequestParser, this->pbuf); | |
35 | + this->writer = new(HttpResponseWriter, this->wbuf); | |
28 | 36 | } |
29 | 37 | |
30 | 38 | static |
... | ... | @@ -33,13 +41,31 @@ dtor(void * _this) |
33 | 41 | { |
34 | 42 | HttpWorker this = _this; |
35 | 43 | |
36 | - free(this->id); | |
44 | + if (NULL != this->id) free(this->id); | |
45 | + | |
46 | + delete(&(this->parser)); | |
47 | + delete(&(this->writer)); | |
48 | + | |
49 | + if (NULL != this->pbuf) delete(&(this->pbuf)); | |
50 | + if (NULL != this->wbuf) delete(&(this->wbuf)); | |
51 | +} | |
52 | + | |
53 | +static | |
54 | +void | |
55 | +_clone(void * _this, void * _base) | |
56 | +{ | |
57 | + HttpWorker this = _this; | |
58 | + HttpWorker base = _base; | |
59 | + | |
60 | + this->id = NULL; | |
61 | + this->pbuf = NULL; | |
62 | + this->wbuf = NULL; | |
37 | 63 | |
38 | - delete(&this->parser); | |
39 | - delete(&this->writer); | |
64 | + this->parser = new(HttpRequestParser, base->pbuf); | |
65 | + this->writer = new(HttpResponseWriter, base->wbuf); | |
40 | 66 | } |
41 | 67 | |
42 | -INIT_IFACE(Class, ctor, dtor, NULL); | |
68 | +INIT_IFACE(Class, ctor, dtor, _clone); | |
43 | 69 | INIT_IFACE(StreamReader, (fptr_streamReaderRead)httpWorkerProcess); |
44 | 70 | INIT_IFACE(StreamWriter, (fptr_streamWriterWrite)httpWorkerWrite); |
45 | 71 | CREATE_CLASS( | ... | ... |
1 | 1 | #include <errno.h> |
2 | 2 | #include <stdio.h> |
3 | +#include <stdlib.h> | |
3 | 4 | |
4 | 5 | #include "http/worker.h" |
5 | 6 | |
... | ... | @@ -16,15 +17,20 @@ serverHandleAccept(Server this) |
16 | 17 | acc = socketAccept(this->sock, &remoteAddr); |
17 | 18 | |
18 | 19 | if (-1 != acc->handle) { |
19 | - char id[21]; | |
20 | - | |
21 | - sprintf(id, "my_%s_%05d", remoteAddr, acc->handle); | |
22 | - | |
23 | 20 | //* save the socket handle |
24 | 21 | (this->conns)[acc->handle].sock = acc; |
25 | 22 | |
26 | - //* clone reader | |
27 | - (this->conns)[acc->handle].worker = new(HttpWorker, id); | |
23 | + //* clone worker | |
24 | + (this->conns)[acc->handle].worker = clone(this->worker); | |
25 | + | |
26 | + /** | |
27 | + * @TODO: | |
28 | + * set worker id---this would better be accomplished | |
29 | + * as a worker method. | |
30 | + * The same is true for server information stuff only | |
31 | + * that this should become an interface. | |
32 | + * At a second thought both has to be an interface. | |
33 | + */ | |
28 | 34 | |
29 | 35 | (this->fds)[this->nfds].fd = acc->handle; |
30 | 36 | (this->fds)[this->nfds].events = POLLIN; | ... | ... |
... | ... | @@ -19,8 +19,8 @@ int |
19 | 19 | main() |
20 | 20 | { |
21 | 21 | Logger logger = new(LoggerSyslog, LOGGER_ERR); |
22 | - //HttpWorker worker = new(HttpWorker); | |
23 | - Server server = new(Server, logger, NULL /*worker*/, 11212, SOMAXCONN); | |
22 | + HttpWorker worker = new(HttpWorker, "my"); | |
23 | + Server server = new(Server, logger, worker, 11212, SOMAXCONN); | |
24 | 24 | |
25 | 25 | struct rlimit limit = {RLIM_INFINITY, RLIM_INFINITY}; |
26 | 26 | setrlimit(RLIMIT_CPU, &limit); |
... | ... | @@ -30,7 +30,7 @@ main() |
30 | 30 | serverRun(server); |
31 | 31 | |
32 | 32 | delete(&server); |
33 | -// delete(&worker); | |
33 | + delete(&worker); | |
34 | 34 | delete(&logger); |
35 | 35 | |
36 | 36 | return 0; | ... | ... |
Please
register
or
login
to post a comment