Merge pull request #27536 from bfredl/bufferarena

refactor(api): reduce temporary allocations when replacing lines
This commit is contained in:
bfredl 2024-02-20 12:19:47 +01:00 committed by GitHub
commit 69bdcc6823
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 73 additions and 80 deletions

View File

@ -325,7 +325,8 @@ ArrayOf(String) nvim_buf_get_lines(uint64_t channel_id,
/// @param replacement Array of lines to use as replacement
/// @param[out] err Error details, if any
void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integer end,
Boolean strict_indexing, ArrayOf(String) replacement, Error *err)
Boolean strict_indexing, ArrayOf(String) replacement, Arena *arena,
Error *err)
FUNC_API_SINCE(1)
FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{
@ -360,14 +361,14 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
size_t new_len = replacement.size;
size_t old_len = (size_t)(end - start);
ptrdiff_t extra = 0; // lines added to text, can be negative
char **lines = (new_len != 0) ? xcalloc(new_len, sizeof(char *)) : NULL;
char **lines = (new_len != 0) ? arena_alloc(arena, new_len * sizeof(char *), true) : NULL;
for (size_t i = 0; i < new_len; i++) {
const String l = replacement.items[i].data.string;
// Fill lines[i] with l's contents. Convert NULs to newlines as required by
// NL-used-for-NUL.
lines[i] = xmemdupz(l.data, l.size);
lines[i] = arena_memdupz(arena, l.data, l.size);
memchrsub(lines[i], NUL, NL, l.size);
}
@ -412,15 +413,12 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
goto end;
});
if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false) == FAIL) {
if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false, true) == FAIL) {
api_set_error(err, kErrorTypeException, "Failed to replace line");
goto end;
}
inserted_bytes += (bcount_t)strlen(lines[i]) + 1;
// Mark lines that haven't been passed to the buffer as they need
// to be freed later
lines[i] = NULL;
}
// Now we may need to insert the remaining new old_len
@ -438,9 +436,6 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
inserted_bytes += (bcount_t)strlen(lines[i]) + 1;
// Same as with replacing, but we also need to free lines
xfree(lines[i]);
lines[i] = NULL;
extra++;
}
@ -463,11 +458,6 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
}
end:
for (size_t i = 0; i < new_len; i++) {
xfree(lines[i]);
}
xfree(lines);
try_end(err);
}
@ -500,7 +490,8 @@ end:
/// @param replacement Array of lines to use as replacement
/// @param[out] err Error details, if any
void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, Integer start_col,
Integer end_row, Integer end_col, ArrayOf(String) replacement, Error *err)
Integer end_row, Integer end_col, ArrayOf(String) replacement, Arena *arena,
Error *err)
FUNC_API_SINCE(7)
FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{
@ -535,33 +526,31 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
return;
});
char *str_at_start = NULL;
char *str_at_end = NULL;
// Another call to ml_get_buf() may free the line, so make a copy.
str_at_start = xstrdup(ml_get_buf(buf, (linenr_T)start_row));
// Another call to ml_get_buf() may free the lines, so we make copies
char *str_at_start = ml_get_buf(buf, (linenr_T)start_row);
size_t len_at_start = strlen(str_at_start);
str_at_start = arena_memdupz(arena, str_at_start, len_at_start);
start_col = start_col < 0 ? (int64_t)len_at_start + start_col + 1 : start_col;
VALIDATE_RANGE((start_col >= 0 && (size_t)start_col <= len_at_start), "start_col", {
goto early_end;
return;
});
// Another call to ml_get_buf() may free the line, so make a copy.
str_at_end = xstrdup(ml_get_buf(buf, (linenr_T)end_row));
char *str_at_end = ml_get_buf(buf, (linenr_T)end_row);
size_t len_at_end = strlen(str_at_end);
str_at_end = arena_memdupz(arena, str_at_end, len_at_end);
end_col = end_col < 0 ? (int64_t)len_at_end + end_col + 1 : end_col;
VALIDATE_RANGE((end_col >= 0 && (size_t)end_col <= len_at_end), "end_col", {
goto early_end;
return;
});
VALIDATE((start_row <= end_row && !(end_row == start_row && start_col > end_col)),
"%s", "'start' is higher than 'end'", {
goto early_end;
return;
});
bool disallow_nl = (channel_id != VIML_INTERNAL_CALL);
if (!check_string_array(replacement, "replacement string", disallow_nl, err)) {
goto early_end;
return;
}
size_t new_len = replacement.size;
@ -591,7 +580,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
if (replacement.size == 1) {
firstlen += last_part_len;
}
char *first = xmallocz(firstlen);
char *first = arena_allocz(arena, firstlen);
char *last = NULL;
memcpy(first, str_at_start, (size_t)start_col);
memcpy(first + start_col, first_item.data, first_item.size);
@ -599,13 +588,13 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
if (replacement.size == 1) {
memcpy(first + start_col + first_item.size, str_at_end + end_col, last_part_len);
} else {
last = xmallocz(last_item.size + last_part_len);
last = arena_allocz(arena, last_item.size + last_part_len);
memcpy(last, last_item.data, last_item.size);
memchrsub(last, NUL, NL, last_item.size);
memcpy(last + last_item.size, str_at_end + end_col, last_part_len);
}
char **lines = xcalloc(new_len, sizeof(char *));
char **lines = arena_alloc(arena, new_len * sizeof(char *), true);
lines[0] = first;
new_byte += (bcount_t)(first_item.size);
for (size_t i = 1; i < new_len - 1; i++) {
@ -613,7 +602,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
// Fill lines[i] with l's contents. Convert NULs to newlines as required by
// NL-used-for-NUL.
lines[i] = xmemdupz(l.data, l.size);
lines[i] = arena_memdupz(arena, l.data, l.size);
memchrsub(lines[i], NUL, NL, l.size);
new_byte += (bcount_t)(l.size) + 1;
}
@ -665,13 +654,10 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
goto end;
});
if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false) == FAIL) {
if (ml_replace_buf(buf, (linenr_T)lnum, lines[i], false, true) == FAIL) {
api_set_error(err, kErrorTypeException, "Failed to replace line");
goto end;
}
// Mark lines that haven't been passed to the buffer as they need
// to be freed later
lines[i] = NULL;
}
// Now we may need to insert the remaining new old_len
@ -687,9 +673,6 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
goto end;
}
// Same as with replacing, but we also need to free lines
xfree(lines[i]);
lines[i] = NULL;
extra++;
}
@ -722,15 +705,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
}
end:
for (size_t i = 0; i < new_len; i++) {
xfree(lines[i]);
}
xfree(lines);
try_end(err);
early_end:
xfree(str_at_start);
xfree(str_at_end);
}
/// Gets a range from the buffer.

View File

@ -225,12 +225,12 @@ Dictionary nvim_get_hl_by_name(String name, Boolean rgb, Arena *arena, Error *er
/// the end of the buffer.
/// @param lines Array of lines
/// @param[out] err Error details, if any
void buffer_insert(Buffer buffer, Integer lnum, ArrayOf(String) lines, Error *err)
void buffer_insert(Buffer buffer, Integer lnum, ArrayOf(String) lines, Arena *arena, Error *err)
FUNC_API_DEPRECATED_SINCE(1)
{
// "lnum" will be the index of the line after inserting,
// no matter if it is negative or not
nvim_buf_set_lines(0, buffer, lnum, lnum, true, lines, err);
nvim_buf_set_lines(0, buffer, lnum, lnum, true, lines, arena, err);
}
/// Gets a buffer line
@ -272,13 +272,13 @@ String buffer_get_line(Buffer buffer, Integer index, Arena *arena, Error *err)
/// @param index Line index
/// @param line Contents of the new line
/// @param[out] err Error details, if any
void buffer_set_line(Buffer buffer, Integer index, String line, Error *err)
void buffer_set_line(Buffer buffer, Integer index, String line, Arena *arena, Error *err)
FUNC_API_DEPRECATED_SINCE(1)
{
Object l = STRING_OBJ(line);
Array array = { .items = &l, .size = 1 };
index = convert_index(index);
nvim_buf_set_lines(0, buffer, index, index + 1, true, array, err);
nvim_buf_set_lines(0, buffer, index, index + 1, true, array, arena, err);
}
/// Deletes a buffer line
@ -291,12 +291,12 @@ void buffer_set_line(Buffer buffer, Integer index, String line, Error *err)
/// @param buffer buffer handle
/// @param index line index
/// @param[out] err Error details, if any
void buffer_del_line(Buffer buffer, Integer index, Error *err)
void buffer_del_line(Buffer buffer, Integer index, Arena *arena, Error *err)
FUNC_API_DEPRECATED_SINCE(1)
{
Array array = ARRAY_DICT_INIT;
index = convert_index(index);
nvim_buf_set_lines(0, buffer, index, index + 1, true, array, err);
nvim_buf_set_lines(0, buffer, index, index + 1, true, array, arena, err);
}
/// Retrieves a line range from the buffer
@ -342,12 +342,13 @@ ArrayOf(String) buffer_get_line_slice(Buffer buffer,
// array will delete the line range)
/// @param[out] err Error details, if any
void buffer_set_line_slice(Buffer buffer, Integer start, Integer end, Boolean include_start,
Boolean include_end, ArrayOf(String) replacement, Error *err)
Boolean include_end, ArrayOf(String) replacement, Arena *arena,
Error *err)
FUNC_API_DEPRECATED_SINCE(1)
{
start = convert_index(start) + !include_start;
end = convert_index(end) + include_end;
nvim_buf_set_lines(0, buffer, start, end, false, replacement, err);
nvim_buf_set_lines(0, buffer, start, end, false, replacement, arena, err);
}
/// Sets a buffer-scoped (b:) variable

View File

@ -675,21 +675,21 @@ String nvim_get_current_line(Arena *arena, Error *err)
///
/// @param line Line contents
/// @param[out] err Error details, if any
void nvim_set_current_line(String line, Error *err)
void nvim_set_current_line(String line, Arena *arena, Error *err)
FUNC_API_SINCE(1)
FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{
buffer_set_line(curbuf->handle, curwin->w_cursor.lnum - 1, line, err);
buffer_set_line(curbuf->handle, curwin->w_cursor.lnum - 1, line, arena, err);
}
/// Deletes the current line.
///
/// @param[out] err Error details, if any
void nvim_del_current_line(Error *err)
void nvim_del_current_line(Arena *arena, Error *err)
FUNC_API_SINCE(1)
FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{
buffer_del_line(curbuf->handle, curwin->w_cursor.lnum - 1, err);
buffer_del_line(curbuf->handle, curwin->w_cursor.lnum - 1, arena, err);
}
/// Gets a global (g:) variable.

View File

@ -4674,7 +4674,7 @@ static int show_sub(exarg_T *eap, pos_T old_cusr, PreviewLines *preview_lines, i
snprintf(str, line_size, "|%*" PRIdLINENR "| %s", col_width - 3,
next_linenr, line);
if (linenr_preview == 0) {
ml_replace_buf(cmdpreview_buf, 1, str, true);
ml_replace_buf(cmdpreview_buf, 1, str, true, false);
} else {
ml_append_buf(cmdpreview_buf, linenr_preview, str, (colnr_T)line_size, false);
}

View File

@ -1626,7 +1626,7 @@ static void foldAddMarker(buf_T *buf, pos_T pos, const char *marker, size_t mark
STRCPY(newline + line_len + (p - cms) + markerlen, p + 2);
added = markerlen + strlen(cms) - 2;
}
ml_replace_buf(buf, lnum, newline, false);
ml_replace_buf(buf, lnum, newline, false, false);
if (added) {
extmark_splice_cols(buf, (int)lnum - 1, (int)line_len,
0, (int)added, kExtmarkUndo);
@ -1690,7 +1690,7 @@ static void foldDelMarker(buf_T *buf, linenr_T lnum, char *marker, size_t marker
assert(p >= line);
memcpy(newline, line, (size_t)(p - line));
STRCPY(newline + (p - line), p + len);
ml_replace_buf(buf, lnum, newline, false);
ml_replace_buf(buf, lnum, newline, false, false);
extmark_splice_cols(buf, (int)lnum - 1, (int)(p - line),
(int)len, 0, kExtmarkUndo);
}

View File

@ -1694,7 +1694,7 @@ void ml_sync_all(int check_file, int check_char, bool do_fsync)
if (buf->b_ml.ml_mfp == NULL || buf->b_ml.ml_mfp->mf_fname == NULL) {
continue; // no file
}
ml_flush_line(buf); // flush buffered line
ml_flush_line(buf, false); // flush buffered line
// flush locked block
ml_find_line(buf, 0, ML_FLUSH);
if (bufIsChanged(buf) && check_file && mf_need_trans(buf->b_ml.ml_mfp)
@ -1745,7 +1745,7 @@ void ml_preserve(buf_T *buf, bool message, bool do_fsync)
// before.
got_int = false;
ml_flush_line(buf); // flush buffered line
ml_flush_line(buf, false); // flush buffered line
ml_find_line(buf, 0, ML_FLUSH); // flush locked block
int status = mf_sync(mfp, MFS_ALL | (do_fsync ? MFS_FLUSH : 0));
@ -1862,7 +1862,7 @@ static char *ml_get_buf_impl(buf_T *buf, linenr_T lnum, bool will_change)
siemsg(_(e_ml_get_invalid_lnum_nr), (int64_t)lnum);
recursive--;
}
ml_flush_line(buf);
ml_flush_line(buf, false);
errorret:
STRCPY(questions, "???");
buf->b_ml.ml_line_lnum = lnum;
@ -1881,7 +1881,7 @@ errorret:
// Don't use the last used line when 'swapfile' is reset, need to load all
// blocks.
if (buf->b_ml.ml_line_lnum != lnum) {
ml_flush_line(buf);
ml_flush_line(buf, false);
// Find the data block containing the line.
// This also fills the stack with the blocks from the root to the data
@ -1964,7 +1964,7 @@ int ml_append(linenr_T lnum, char *line, colnr_T len, bool newfile)
}
if (curbuf->b_ml.ml_line_lnum != 0) {
ml_flush_line(curbuf);
ml_flush_line(curbuf, false);
}
return ml_append_int(curbuf, lnum, line, len, newfile, false);
}
@ -1984,7 +1984,7 @@ int ml_append_buf(buf_T *buf, linenr_T lnum, char *line, colnr_T len, bool newfi
}
if (buf->b_ml.ml_line_lnum != 0) {
ml_flush_line(buf);
ml_flush_line(buf, false);
}
return ml_append_int(buf, lnum, line, len, newfile, false);
}
@ -2423,7 +2423,7 @@ void ml_add_deleted_len_buf(buf_T *buf, char *ptr, ssize_t len)
int ml_replace(linenr_T lnum, char *line, bool copy)
{
return ml_replace_buf(curbuf, lnum, line, copy);
return ml_replace_buf(curbuf, lnum, line, copy, false);
}
/// Replace line "lnum", with buffering, in current buffer.
@ -2438,7 +2438,7 @@ int ml_replace(linenr_T lnum, char *line, bool copy)
/// changed_lines(), unless update_screen(UPD_NOT_VALID) is used.
///
/// @return FAIL for failure, OK otherwise
int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy)
int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy, bool noalloc)
{
if (line == NULL) { // just checking...
return FAIL;
@ -2450,12 +2450,13 @@ int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy)
}
if (copy) {
assert(!noalloc);
line = xstrdup(line);
}
if (buf->b_ml.ml_line_lnum != lnum) {
// another line is buffered, flush it
ml_flush_line(buf);
ml_flush_line(buf, false);
}
if (kv_size(buf->update_callbacks)) {
@ -2469,6 +2470,13 @@ int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy)
buf->b_ml.ml_line_ptr = line;
buf->b_ml.ml_line_lnum = lnum;
buf->b_ml.ml_flags = (buf->b_ml.ml_flags | ML_LINE_DIRTY) & ~ML_EMPTY;
if (noalloc) {
// TODO(bfredl): this is a bit of a hack. but replacing lines in a loop is really common,
// and allocating a separate scratch buffer for each line which is immediately freed adds
// a lot of noise. A more general refactor could be to use a _fixed_ scratch buffer for
// all lines up to $REASONABLE_SIZE .
ml_flush_line(buf, true);
}
return OK;
}
@ -2483,7 +2491,7 @@ int ml_replace_buf(buf_T *buf, linenr_T lnum, char *line, bool copy)
/// @return FAIL for failure, OK otherwise
int ml_delete(linenr_T lnum, bool message)
{
ml_flush_line(curbuf);
ml_flush_line(curbuf, false);
return ml_delete_int(curbuf, lnum, message);
}
@ -2496,7 +2504,7 @@ int ml_delete(linenr_T lnum, bool message)
/// @return FAIL for failure, OK otherwise
int ml_delete_buf(buf_T *buf, linenr_T lnum, bool message)
{
ml_flush_line(buf);
ml_flush_line(buf, false);
return ml_delete_int(buf, lnum, message);
}
@ -2516,7 +2524,7 @@ static int ml_delete_int(buf_T *buf, linenr_T lnum, bool message)
set_keep_msg(_(no_lines_msg), 0);
}
int i = ml_replace_buf(buf, 1, "", true);
int i = ml_replace_buf(buf, 1, "", true, false);
buf->b_ml.ml_flags |= ML_EMPTY;
return i;
@ -2724,7 +2732,7 @@ size_t ml_flush_deleted_bytes(buf_T *buf, size_t *codepoints, size_t *codeunits)
}
/// flush ml_line if necessary
static void ml_flush_line(buf_T *buf)
static void ml_flush_line(buf_T *buf, bool noalloc)
{
static bool entered = false;
@ -2798,10 +2806,13 @@ static void ml_flush_line(buf_T *buf)
ml_delete_int(buf, lnum, false);
}
}
if (!noalloc) {
xfree(new_line);
}
entered = false;
} else if (buf->b_ml.ml_flags & ML_ALLOCATED) {
assert(!noalloc); // caller must set ML_LINE_DIRTY with noalloc, handled above
xfree(buf->b_ml.ml_line_ptr);
}
@ -3903,7 +3914,7 @@ int ml_find_line_or_offset(buf_T *buf, linenr_T lnum, int *offp, bool no_ff)
// was never cached to start with anyway).
bool can_cache = (lnum != 0 && !ffdos && buf->b_ml.ml_line_lnum == lnum);
if (lnum == 0 || buf->b_ml.ml_line_lnum < lnum || !no_ff) {
ml_flush_line(curbuf);
ml_flush_line(curbuf, false);
} else if (can_cache && buf->b_ml.ml_line_offset > 0) {
return (int)buf->b_ml.ml_line_offset;
}
@ -4030,7 +4041,7 @@ void goto_byte(int cnt)
{
int boff = cnt;
ml_flush_line(curbuf); // cached line may be dirty
ml_flush_line(curbuf, false); // cached line may be dirty
setpcmark();
if (boff) {
boff--;

View File

@ -667,14 +667,20 @@ void arena_mem_free(ArenaMem mem)
}
}
char *arena_memdupz(Arena *arena, const char *buf, size_t size)
char *arena_allocz(Arena *arena, size_t size)
{
char *mem = arena_alloc(arena, size + 1, false);
memcpy(mem, buf, size);
mem[size] = NUL;
return mem;
}
char *arena_memdupz(Arena *arena, const char *buf, size_t size)
{
char *mem = arena_allocz(arena, size);
memcpy(mem, buf, size);
return mem;
}
#if defined(EXITFREE)
# include "nvim/autocmd.h"

View File

@ -581,7 +581,7 @@ Array runtime_inspect(Arena *arena)
for (size_t i = 0; i < kv_size(path); i++) {
SearchPathItem *item = &kv_A(path, i);
Array entry = ARRAY_DICT_INIT;
Array entry = arena_array(arena, 3);
ADD_C(entry, CSTR_AS_OBJ(item->path));
ADD_C(entry, BOOLEAN_OBJ(item->after));
if (item->has_lua != kNone) {