perf(memory): use an arena for RPC decoding

drawback: tracing memory errors with ASAN is less accurate for arena
allocated memory.
Therefore, to start with it is being used for Object types around
serialization/deserialization exclusively. This is going to have
a large impact especially when TUI is refactored as a co-prosess
as all UI events will be serialized and deserialized by nvim itself.
This commit is contained in:
bfredl
2022-06-02 09:18:13 +02:00
parent 7f8f8d6cb7
commit 3f5c647de9
10 changed files with 158 additions and 33 deletions

View File

@@ -26,6 +26,7 @@
#include "nvim/message.h"
#include "nvim/msgpack_rpc/channel.h"
#include "nvim/msgpack_rpc/helpers.h"
#include "nvim/msgpack_rpc/unpacker.h"
#include "nvim/os/input.h"
#include "nvim/os_unix.h"
#include "nvim/ui.h"
@@ -113,7 +114,8 @@ bool rpc_send_event(uint64_t id, const char *name, Array args)
/// @param args Array with method arguments
/// @param[out] error True if the return value is an error
/// @return Whatever the remote method returned
Object rpc_send_call(uint64_t id, const char *method_name, Array args, Error *err)
Object rpc_send_call(uint64_t id, const char *method_name, Array args, ArenaMem *result_mem,
Error *err)
{
Channel *channel = NULL;
@@ -130,7 +132,7 @@ Object rpc_send_call(uint64_t id, const char *method_name, Array args, Error *er
send_request(channel, request_id, method_name, args);
// Push the frame
ChannelCallFrame frame = { request_id, false, false, NIL };
ChannelCallFrame frame = { request_id, false, false, NIL, NULL };
kv_push(rpc->call_stack, &frame);
LOOP_PROCESS_EVENTS_UNTIL(&main_loop, channel->events, -1, frame.returned);
(void)kv_pop(rpc->call_stack);
@@ -155,11 +157,15 @@ Object rpc_send_call(uint64_t id, const char *method_name, Array args, Error *er
api_set_error(err, kErrorTypeException, "%s", "unknown error");
}
api_free_object(frame.result);
// frame.result was allocated in an arena
arena_mem_free(frame.result_mem, &rpc->unpacker->reuse_blk);
frame.result_mem = NULL;
}
channel_decref(channel);
*result_mem = frame.result_mem;
return frame.errored ? NIL : frame.result;
}
@@ -249,17 +255,17 @@ static void parse_msgpack(Channel *channel)
if (frame->errored) {
frame->result = p->error;
// TODO(bfredl): p->result should not even be decoded
api_free_object(p->result);
// api_free_object(p->result);
} else {
frame->result = p->result;
}
frame->result_mem = arena_finish(&p->arena);
} else {
log_client_msg(channel->id, p->type == kMessageTypeRequest, p->handler.name);
Object res = p->result;
if (p->result.type != kObjectTypeArray) {
chan_close_with_error(channel, "msgpack-rpc request args has to be an array", LOGLVL_ERR);
api_free_object(p->result);
return;
}
Array arg = res.data.array;
@@ -282,7 +288,7 @@ static void handle_request(Channel *channel, Unpacker *p, Array args)
if (!p->handler.fn) {
send_error(channel, p->type, p->request_id, p->unpack_error.msg);
api_clear_error(&p->unpack_error);
api_free_array(args);
arena_mem_free(arena_finish(&p->arena), &p->reuse_blk);
return;
}
@@ -291,6 +297,7 @@ static void handle_request(Channel *channel, Unpacker *p, Array args)
evdata->channel = channel;
evdata->handler = p->handler;
evdata->args = args;
evdata->used_mem = arena_finish(&p->arena);
evdata->request_id = p->request_id;
channel_incref(channel);
if (p->handler.fast) {
@@ -346,7 +353,8 @@ static void request_event(void **argv)
}
free_ret:
api_free_array(e->args);
// e->args is allocated in an arena
arena_mem_free(e->used_mem, &channel->rpc.unpacker->reuse_blk);
channel_decref(channel);
xfree(e);
api_clear_error(&error);
@@ -523,6 +531,7 @@ static void exit_event(void **argv)
void rpc_free(Channel *channel)
{
remote_ui_disconnect(channel->id);
unpacker_teardown(channel->rpc.unpacker);
xfree(channel->rpc.unpacker);
// Unsubscribe from all events
@@ -543,7 +552,6 @@ static void chan_close_with_error(Channel *channel, char *msg, int loglevel)
ChannelCallFrame *frame = kv_A(channel->rpc.call_stack, i);
frame->returned = true;
frame->errored = true;
api_free_object(frame->result);
frame->result = STRING_OBJ(cstr_to_string(msg));
}

View File

@@ -9,15 +9,16 @@
#include "nvim/api/private/dispatch.h"
#include "nvim/event/process.h"
#include "nvim/event/socket.h"
#include "nvim/msgpack_rpc/unpacker.h"
#include "nvim/vim.h"
typedef struct Channel Channel;
typedef struct Unpacker Unpacker;
typedef struct {
uint32_t request_id;
bool returned, errored;
Object result;
ArenaMem result_mem;
} ChannelCallFrame;
typedef struct {
@@ -26,6 +27,7 @@ typedef struct {
MsgpackRpcRequestHandler handler;
Array args;
uint32_t request_id;
ArenaMem used_mem;
} RequestEvent;
typedef struct {

View File

@@ -3,6 +3,7 @@
#include "nvim/api/private/helpers.h"
#include "nvim/log.h"
#include "nvim/memory.h"
#include "nvim/msgpack_rpc/helpers.h"
#include "nvim/msgpack_rpc/unpacker.h"
@@ -10,6 +11,10 @@
# include "msgpack_rpc/unpacker.c.generated.h"
#endif
#define kv_fixsize_arena(a, v, s) \
((v).capacity = (s), \
(v).items = (void *)arena_alloc(a, sizeof((v).items[0]) * (v).capacity, true))
Object unpack(const char *data, size_t size, Error *err)
{
Unpacker unpacker;
@@ -34,7 +39,7 @@ Object unpack(const char *data, size_t size, Error *err)
static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
{
Unpacker *unpacker = parser->data.p;
Unpacker *p = parser->data.p;
Object *result = NULL;
String *key_location = NULL;
@@ -69,7 +74,7 @@ static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
abort();
}
} else {
result = &unpacker->result;
result = &p->result;
}
switch (node->tok.type) {
@@ -88,16 +93,17 @@ static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
case MPACK_TOKEN_FLOAT:
*result = FLOAT_OBJ(mpack_unpack_float(node->tok));
break;
case MPACK_TOKEN_BIN:
case MPACK_TOKEN_STR: {
String str = { .data = xmallocz(node->tok.length), .size = node->tok.length };
char *mem = arena_alloc(&p->arena, node->tok.length + 1, false);
mem[node->tok.length] = NUL;
String str = { .data = mem, .size = node->tok.length };
if (key_location) {
*key_location = str;
} else {
*result = STRING_OBJ(str);
}
node->data[0].p = str.data;
break;
}
@@ -105,7 +111,6 @@ static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
// handled in chunk; but save result location
node->data[0].p = result;
break;
case MPACK_TOKEN_CHUNK:
assert(parent);
if (parent->tok.type == MPACK_TOKEN_STR || parent->tok.type == MPACK_TOKEN_BIN) {
@@ -120,12 +125,12 @@ static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
*res = NIL;
break;
}
memcpy(unpacker->ext_buf + parent->pos,
memcpy(p->ext_buf + parent->pos,
node->tok.data.chunk_ptr, node->tok.length);
if (parent->pos + node->tok.length < parent->tok.length) {
break; // EOF, let's get back to it later
}
const char *buf = unpacker->ext_buf;
const char *buf = p->ext_buf;
size_t size = parent->tok.length;
mpack_token_t ext_tok;
int status = mpack_rtoken(&buf, &size, &ext_tok);
@@ -148,7 +153,7 @@ static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
case MPACK_TOKEN_ARRAY: {
Array arr = KV_INITIAL_VALUE;
kv_resize(arr, node->tok.length);
kv_fixsize_arena(&p->arena, arr, node->tok.length);
kv_size(arr) = node->tok.length;
*result = ARRAY_OBJ(arr);
node->data[0].p = result;
@@ -156,12 +161,13 @@ static void api_parse_enter(mpack_parser_t *parser, mpack_node_t *node)
}
case MPACK_TOKEN_MAP: {
Dictionary dict = KV_INITIAL_VALUE;
kv_resize(dict, node->tok.length);
kv_fixsize_arena(&p->arena, dict, node->tok.length);
kv_size(dict) = node->tok.length;
*result = DICTIONARY_OBJ(dict);
node->data[0].p = result;
break;
}
default:
abort();
}
@@ -176,6 +182,15 @@ void unpacker_init(Unpacker *p)
p->parser.data.p = p;
mpack_tokbuf_init(&p->reader);
p->unpack_error = (Error)ERROR_INIT;
p->arena = (Arena)ARENA_EMPTY;
p->reuse_blk = NULL;
}
void unpacker_teardown(Unpacker *p)
{
arena_mem_free(p->reuse_blk, NULL);
arena_mem_free(arena_finish(&p->arena), NULL);
}
bool unpacker_parse_header(Unpacker *p)
@@ -282,6 +297,7 @@ bool unpacker_advance(Unpacker *p)
return false;
}
p->state = p->type == kMessageTypeResponse ? 1 : 2;
arena_start(&p->arena, &p->reuse_blk);
}
int result;

View File

@@ -9,8 +9,10 @@
#include "mpack/object.h"
#include "nvim/api/private/dispatch.h"
#include "nvim/api/private/helpers.h"
#include "nvim/memory.h"
#include "nvim/msgpack_rpc/channel_defs.h"
typedef struct {
struct Unpacker {
mpack_parser_t parser;
mpack_tokbuf_t reader;
@@ -28,7 +30,11 @@ typedef struct {
Object error; // error return
Object result; // arg list or result
Error unpack_error;
} Unpacker;
Arena arena;
// one lenght free-list of reusable blocks
ArenaMem reuse_blk;
};
// unrecovareble error. unpack_error should be set!
#define unpacker_closed(p) ((p)->state < 0)