Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions crates/edit/src/buffer/gap_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,39 @@ impl GapBuffer {
})
}

/// Enlarges the reserved capacity of the buffer.
///
/// TODO: Ideally we would only lazily reserve virtual memory, such that this doesn't
/// need to reserve + release. However, this requires reporting errors from enlarge_gap.
pub fn try_reserve(&mut self, bytes: usize) {
if bytes < self.reserve {
return;
}

if self.text_length != 0 {
debug_assert!(false);
return;
}
let BackingBuffer::VirtualMemory(old_ptr, old_len) = self.buffer else {
debug_assert!(false);
return;
};

unsafe {
let bytes =
bytes.saturating_add(MEBI + LARGE_ALLOC_CHUNK - 1) & !(LARGE_ALLOC_CHUNK - 1);
if let Ok(ptr) = virtual_reserve(bytes) {
virtual_release(old_ptr, old_len);
self.buffer = BackingBuffer::VirtualMemory(ptr, bytes);
self.text = ptr;
self.reserve = bytes;
self.commit = 0;
self.gap_off = 0;
self.gap_len = 0;
}
}
}

#[allow(clippy::len_without_is_empty)]
pub fn len(&self) -> usize {
self.text_length
Expand Down Expand Up @@ -179,17 +212,14 @@ impl GapBuffer {

let gap_len_old = self.gap_len;
let gap_len_new = (len + gap_chunk + gap_chunk - 1) & !(gap_chunk - 1);
let gap_len_new = gap_len_new.min(self.reserve - self.text_length);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change (and the removed if condition below), reading files larger than the max. memory would read in chunks of 4KiB.


let bytes_old = self.commit;
let bytes_new = self.text_length + gap_len_new;

if bytes_new > bytes_old {
let bytes_new = (bytes_new + alloc_chunk - 1) & !(alloc_chunk - 1);

if bytes_new > self.reserve {
return;
}

match &mut self.buffer {
BackingBuffer::VirtualMemory(ptr, _) => unsafe {
if virtual_commit(ptr.add(bytes_old), bytes_new - bytes_old).is_err() {
Expand Down
45 changes: 28 additions & 17 deletions crates/edit/src/buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,23 @@ impl TextBuffer {
first_chunk_len: usize,
done: bool,
) -> io::Result<()> {
// Get the length of the file. 0 = not a file.
let file_len = if done {
// But if the first 4KiB read already contains the entire file, we won't need
// the file length below (we early return). The value here doesn't matter.
0
} else {
// We can't acquire the length on pipes, for instance.
file.metadata().ok().and_then(|m| m.len().try_into().ok()).unwrap_or(0)
};
Comment on lines +856 to +864

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this and the changes below prevent that we ever (typically) run into these out of memory situations in the first place.
It fixes the issue from both sides!


// If we have a file length, reserve enough space for it.
// The call is a no-op for small files (currently <4GiB).
if file_len > 0 {
self.buffer.try_reserve(file_len);
}

// Handle the first chunk we already read for encoding detection.
{
let mut first_chunk = unsafe { buf[..first_chunk_len].assume_init_ref() };
if first_chunk.starts_with(b"\xEF\xBB\xBF") {
Expand All @@ -862,27 +879,22 @@ impl TextBuffer {

self.buffer.replace(0..0, first_chunk);
}

if done {
return Ok(());
}

// If we don't have file metadata, the input may be a pipe or a socket.
// Every read will have the same size until we hit the end.
let mut chunk_size = 128 * KIBI;
let mut extra_chunk_size = 128 * KIBI;

if let Ok(m) = file.metadata() {
// Usually the next read of size `chunk_size` will read the entire file,
// but if the size has changed for some reason, then `extra_chunk_size`
// should be large enough to read the rest of the file.
// 4KiB is not too large and not too slow.
let len = m.len() as usize;
chunk_size = len.saturating_sub(first_chunk_len);
extra_chunk_size = 4 * KIBI;
}

loop {
let chunk_size = if file_len > 0 {
// If we know the file length:
// * Read the file until the end
// * And if we're still reading at that point, read in 4KiB chunks (e.g. if someone wrote
// to the file concurrently; typically this won't happen, so the chunk size is small).
file_len.checked_sub(self.text_length()).unwrap_or(4 * KIBI)
} else {
// For pipes, sockets, etc., read in 128KiB chunks, because anything smaller has poor perf.
128 * KIBI
};

let gap = self.buffer.allocate_gap(self.text_length(), chunk_size, 0);
if gap.is_empty() {
break;
Expand All @@ -894,7 +906,6 @@ impl TextBuffer {
}

self.buffer.commit_gap(read);
chunk_size = extra_chunk_size;
}

Ok(())
Expand Down