From 7dae9cc3051478a383b52d058730aa831279977b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 16:36:39 +0000 Subject: [PATCH] Load into the piece table's original store; save and copy without materializing A loaded file's bytes are adopted as the piece table's original storage instead of being appended to the add buffer: the first keystroke no longer reallocates and copies the whole file (140 ms at 200 MB), and the read buffer is not copied again. Save writes the pieces in order instead of materializing the buffer (a second full copy that then stayed resident). Buffer::Bytes() assembles a copy from the pieces and replaces four hand-rolled copies (swap snapshots, recovery comparison, Command.cc's per-row join, the tests' accessor). A single-piece table (an unedited file) is viewed in place; GetLineView serves a line from its piece when it lies within one, so it no longer copies the whole buffer after every edit. Code that needed the whole text contiguous says so with ContentView(). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- Buffer.cc | 112 ++++++++++++++++++++++++-------------------------- Buffer.h | 21 ++++++++-- Command.cc | 28 ++----------- Editor.cc | 17 +------- PieceTable.cc | 47 +++++++++++++++++++++ PieceTable.h | 40 ++++++++++++++---- Swap.cc | 20 +-------- 7 files changed, 156 insertions(+), 129 deletions(-) diff --git a/Buffer.cc b/Buffer.cc index ccc1265..1b4a473 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -99,19 +99,26 @@ Buffer::RefreshOnDiskIdentity() } +// The bytes to save, as views of the piece table's storage in order (saving +// never makes a contiguous copy of the buffer). +using ByteChunks = std::vector; + + static bool -write_all_fd(int fd, const char *data, std::size_t len, std::string &err) +write_all_fd(int fd, const ByteChunks &chunks, std::string &err) { - std::size_t off = 0; - while (off < len) { - ssize_t n = ::write(fd, data + off, len - off); - if (n < 0) { - if (errno == EINTR) - continue; - err = std::string("Write failed: ") + std::strerror(errno); - return false; + for (const std::string_view chunk: chunks) { + std::size_t off = 0; + while (off < chunk.size()) { + ssize_t n = ::write(fd, chunk.data() + off, chunk.size() - off); + if (n < 0) { + if (errno == EINTR) + continue; + err = std::string("Write failed: ") + std::strerror(errno); + return false; + } + off += static_cast(n); } - off += static_cast(n); } return true; } @@ -149,7 +156,7 @@ static const mode_t g_process_umask = [] { // Rewrite `path` in place (truncate, write, fsync). Used for files with // several hard links, which a temp-file-and-rename save would split. static bool -write_in_place(const std::string &path, const char *data, std::size_t len, std::string &err) +write_in_place(const std::string &path, const ByteChunks &data, std::string &err) { int flags = O_WRONLY | O_TRUNC; #ifdef O_CLOEXEC @@ -160,7 +167,7 @@ write_in_place(const std::string &path, const char *data, std::size_t len, std:: err = std::string("Failed to open file for writing: ") + std::strerror(errno); return false; } - bool ok = write_all_fd(fd, data, len, err); + bool ok = write_all_fd(fd, data, err); if (ok && kte::syscall::Fsync(fd) != 0) { err = std::string("fsync failed: ") + std::strerror(errno); ok = false; @@ -173,7 +180,7 @@ write_in_place(const std::string &path, const char *data, std::size_t len, std:: // Create a new file named from `tmpl` (a mkstemp template, which receives // the chosen name) holding the content, fsynced. static bool -write_new_file(std::string &tmpl, const char *data, std::size_t len, std::string &err, int &create_errno) +write_new_file(std::string &tmpl, const ByteChunks &data, std::string &err, int &create_errno) { create_errno = 0; std::vector name(tmpl.begin(), tmpl.end()); @@ -186,7 +193,7 @@ write_new_file(std::string &tmpl, const char *data, std::size_t len, std::string } tmpl.assign(name.data()); const std::string &path = tmpl; - bool ok = write_all_fd(fd, data, len, err); + bool ok = write_all_fd(fd, data, err); if (ok && kte::syscall::Fsync(fd) != 0) { err = std::string("fsync failed: ") + std::strerror(errno); ok = false; @@ -211,7 +218,7 @@ may_write_in_place_after(int create_errno) static bool -atomic_write_file(const std::string &path_in, const char *data, std::size_t len, std::string &err) +atomic_write_file(const std::string &path_in, const ByteChunks &data, std::string &err) { // Write through a symlink to its target: renaming over the link itself // would replace the link with a regular file. @@ -242,12 +249,12 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len, std::string copy = path + ".kte-save.XXXXXX"; std::string copy_err; int copy_errno = 0; - const bool have_copy = write_new_file(copy, data, len, copy_err, copy_errno); + const bool have_copy = write_new_file(copy, data, copy_err, copy_errno); if (!have_copy && !may_write_in_place_after(copy_errno)) { err = "Save failed, file left unchanged: " + copy_err; return false; } - if (!write_in_place(path, data, len, err)) { + if (!write_in_place(path, data, err)) { if (have_copy) err += " (the new content is in " + copy + ")"; return false; @@ -287,7 +294,7 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len, // the like: the in-place write would truncate the file and then fail. if (fd < 0 && dst_exists && S_ISREG(dst_st.st_mode) && may_write_in_place_after(saved_errno)) { err.clear(); - return write_in_place(path, data, len, err); + return write_in_place(path, data, err); } if (fd < 0) { err = std::string("Failed to create temp file for save: ") + std::strerror(saved_errno) + err; @@ -309,7 +316,7 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len, (void) kte::syscall::Fchmod(fd, 0666 & ~g_process_umask); } - bool ok = write_all_fd(fd, data, len, err); + bool ok = write_all_fd(fd, data, err); // Never retry fsync: after a writeback error Linux may mark the pages // clean, so a second fsync can succeed although the data was lost, and // the rename below would then replace a good file with a bad one. @@ -654,17 +661,8 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) // Build the new content aside and swap it in only once complete: if the // copy runs out of memory, the buffer keeps its old text (clearing first // left an empty buffer that a later save wrote over the file). - PieceTable fresh; - try { - if (!data.empty()) - fresh.Append(data.data(), data.size()); - std::string().swap(data); - } catch (const std::bad_alloc &) { - err = "File too large to load: " + norm; - kte::ErrorHandler::Instance().Error("Buffer", err, norm); - return false; - } - content_ = std::move(fresh); + // The bytes become the piece table's original storage, uncopied. + content_.AdoptOriginal(std::move(data)); rows_cache_dirty_ = true; nrows_ = 0; // not used under PieceTable filename_ = norm; @@ -699,13 +697,11 @@ Buffer::Save(std::string &err) const err = "Buffer is not file-backed; use SaveAs()"; return false; } - const std::size_t sz = content_.Size(); - const char *data = sz ? content_.Data() : nullptr; - if (sz && !data) { - err = "Internal error: buffer materialization failed"; - return false; - } - if (!atomic_write_file(filename_, data ? data : "", sz, err)) { + ByteChunks chunks; + content_.ForEachChunk([&](const char *d, std::size_t n) { + chunks.emplace_back(d, n); + }); + if (!atomic_write_file(filename_, chunks, err)) { kte::ErrorHandler::Instance().Error("Buffer", err, filename_); return false; } @@ -739,13 +735,11 @@ Buffer::SaveAs(const std::string &path, std::string &err) out_path = path; } - const std::size_t sz = content_.Size(); - const char *data = sz ? content_.Data() : nullptr; - if (sz && !data) { - err = "Internal error: buffer materialization failed"; - return false; - } - if (!atomic_write_file(out_path, data ? data : "", sz, err)) { + ByteChunks chunks; + content_.ForEachChunk([&](const char *d, std::size_t n) { + chunks.emplace_back(d, n); + }); + if (!atomic_write_file(out_path, chunks, err)) { kte::ErrorHandler::Instance().Error("Buffer", err, out_path); return false; } @@ -793,17 +787,23 @@ Buffer::insert_text(int row, int col, std::string_view text) // ===== Adapter helpers for PieceTable-backed Buffer ===== +std::string +Buffer::Bytes() const +{ + std::string out; + out.reserve(content_.Size()); + content_.ForEachChunk([&](const char *d, std::size_t n) { + out.append(d, n); + }); + return out; +} + + std::string_view Buffer::GetLineView(std::size_t row) const { - // Get byte range for the logical line and return a view into materialized data - auto range = content_.GetLineRange(row); // [start,end) in bytes - const char *base = content_.Data(); // materializes if needed - if (!base) - return std::string_view(); - const std::size_t start = range.first; - const std::size_t len = (range.second > range.first) ? (range.second - range.first) : 0; - return std::string_view(base + start, len); + const auto [start, end] = content_.GetLineRange(row); // [start,end) in bytes + return end > start ? content_.View(start, end - start) : std::string_view(); } @@ -836,13 +836,7 @@ Buffer::content_LineCount_() const std::string Buffer::BytesForTests() const { - const std::size_t sz = content_.Size(); - if (sz == 0) - return std::string(); - const char *data = content_.Data(); - if (!data) - return std::string(); - return std::string(data, data + sz); + return Bytes(); } #endif diff --git a/Buffer.h b/Buffer.h index 2c36ec8..be24ee8 100644 --- a/Buffer.h +++ b/Buffer.h @@ -29,7 +29,7 @@ * * 4. Content access: * - Rows(): Materialized line cache (legacy, being phased out) - * - GetLineView(): Zero-copy line access via string_view (preferred) + * - GetLineView(): line access via string_view (zero-copy within a piece) * - Direct PieceTable access for new editing operations */ #pragma once @@ -311,10 +311,25 @@ public: } - // Zero-copy view of a line. Points into the materialized backing store; becomes - // invalid after subsequent edits. Use immediately. + // View of a line's raw bytes, including its trailing '\n' if any. No copy + // when the line lies within one piece (an unedited stretch); a line that + // straddles an edit materializes the whole buffer, so per-row loops over + // an edited buffer should prefer GetLineString(). Invalid after the next + // edit; views of different lines need not be contiguous. [[nodiscard]] std::string_view GetLineView(std::size_t row) const; + // The whole text as one contiguous view (materializes an edited buffer). + // Invalid after the next edit. + [[nodiscard]] std::string_view ContentView() const + { + return content_.ContentView(); + } + + + // A copy of the whole text, assembled from the pieces (no materialized + // copy is kept). + [[nodiscard]] std::string Bytes() const; + [[nodiscard]] const std::string &Filename() const { diff --git a/Command.cc b/Command.cc index 507d1af..acb2f18 100644 --- a/Command.cc +++ b/Command.cc @@ -638,21 +638,6 @@ delete_region(Buffer &buf, std::size_t sx, std::size_t sy, std::size_t ex, std:: } -// The buffer's full text (rows joined by '\n', which reproduces the bytes). -static std::string -buffer_text(const Buffer &buf) -{ - std::string out; - const std::size_t nrows = buf.Nrows(); - for (std::size_t y = 0; y < nrows; ++y) { - if (y > 0) - out.push_back('\n'); - out += buf.GetLineString(y); - } - return out; -} - - // Replace the text of rows [y, y + old_text lines) in place with new_text, // where old_text is those rows joined by '\n' (no trailing newline). Unlike // delete_row/insert_row this never adds or removes the newline after the @@ -929,16 +914,11 @@ static constexpr std::size_t kSearchCountBytesRegex = std::size_t{4} << 20; static std::string_view line_text_view(const Buffer &buf, std::size_t y); -// The whole text as one view into the materialized buffer. +// The whole text as one view. static std::string_view buffer_text_view(const Buffer &buf) { - const std::size_t nrows = buf.Nrows(); - if (nrows == 0) - return {}; - const char *base = buf.GetLineView(0).data(); - const auto last = buf.GetLineView(nrows - 1); - return {base, static_cast(last.data() + last.size() - base)}; + return buf.ContentView(); } @@ -3187,7 +3167,7 @@ cmd_newline(CommandContext &ctx) // Build the replaced text in one pass and apply it as a single // edit: one piece-table edit per match cost O(file) each, so // replace-all was quadratic (298 s for 249k matches in 12 MB). - const std::string before = buffer_text(*buf); + const std::string before = buf->Bytes(); std::string after; after.reserve(before.size()); std::size_t pos = 0; @@ -3653,7 +3633,7 @@ cmd_newline(CommandContext &ctx) } }); if (changed > 0) - replace_buffer_text(*buf, buffer_text(*buf), after, ru); + replace_buffer_text(*buf, buf->Bytes(), after, ru); clamp_cursor_to_buffer(*buf); buf->SetDirty(true); ctx.editor.SetStatus("Regex replaced in " + std::to_string(changed) + " line(s)" + diff --git a/Editor.cc b/Editor.cc index 022fb95..1ad0133 100644 --- a/Editor.cc +++ b/Editor.cc @@ -18,19 +18,6 @@ constexpr int kMaxUArgCount = 1000000; namespace { -static std::string -buffer_bytes_via_views(const Buffer &b) -{ - const std::size_t nrows = b.Nrows(); - std::string out; - for (std::size_t i = 0; i < nrows; i++) { - auto v = b.GetLineView(i); - out.append(v.data(), v.size()); - } - return out; -} - - static void apply_pending_line(Editor &ed, const std::size_t line1) { @@ -597,11 +584,11 @@ Editor::process_pending_opens_() Buffer tmp; std::string oerr; if (tmp.OpenFromFile(req.path, oerr)) { - const std::string orig = buffer_bytes_via_views(tmp); + const std::string orig = tmp.Bytes(); std::string rerr; std::uint64_t valid = 0; if (kte::SwapManager::ReplayFile(tmp, swp, rerr, &valid)) { - std::string rec = buffer_bytes_via_views(tmp); + std::string rec = tmp.Bytes(); if (rec == orig) { // Nothing to recover. Remove the journal rather than // appending this session's records to it (it may end in a diff --git a/PieceTable.cc b/PieceTable.cc index 71a33f8..6cf7840 100644 --- a/PieceTable.cc +++ b/PieceTable.cc @@ -185,6 +185,7 @@ void PieceTable::Clear() { pieces_.clear(); + std::string().swap(original_); add_.clear(); materialized_.clear(); total_size_ = 0; @@ -198,6 +199,52 @@ PieceTable::Clear() } +void +PieceTable::AdoptOriginal(std::string &&bytes) +{ + Clear(); + std::string().swap(add_); // release a previous content's capacity + std::string().swap(materialized_); + original_ = std::move(bytes); + total_size_ = original_.size(); + if (total_size_ > 0) + pieces_.push_back(Piece{Source::Original, 0, total_size_}); +} + + +std::string_view +PieceTable::ContentView() const +{ + if (total_size_ == 0) + return {}; + if (pieces_.size() == 1) { + const Piece &p = pieces_.front(); + return {(p.src == Source::Original ? original_ : add_).data() + p.start, p.len}; + } + materialize(); + return {materialized_.data(), materialized_.size()}; +} + + +std::string_view +PieceTable::View(std::size_t byte_offset, std::size_t len) const +{ + if (byte_offset >= total_size_ || len == 0) + return {}; + len = std::min(len, total_size_ - byte_offset); + if (dirty_) { + // Materialized text is stale: serve the range from its piece when it + // fits in one, rather than copying the whole buffer. + auto [idx, inner] = locate(byte_offset); + if (idx < pieces_.size() && inner + len <= pieces_[idx].len) { + const Piece &p = pieces_[idx]; + return {(p.src == Source::Original ? original_ : add_).data() + p.start + inner, len}; + } + } + return ContentView().substr(byte_offset, len); +} + + void PieceTable::addPieceBack(const Source src, const std::size_t start, const std::size_t len) { diff --git a/PieceTable.h b/PieceTable.h index c41545a..bfae059 100644 --- a/PieceTable.h +++ b/PieceTable.h @@ -4,7 +4,7 @@ * PieceTable is kte's core text storage data structure. It provides efficient * insert/delete operations without copying the entire buffer by maintaining a * sequence of "pieces" that reference ranges in two underlying buffers: - * - original_: Initial file content (currently unused, reserved for future) + * - original_: A loaded file's content (AdoptOriginal), never modified * - add_: All text added during editing * * Key advantages: @@ -15,7 +15,8 @@ * * Performance characteristics: * - Piece count grows with edit operations; automatic consolidation prevents unbounded growth - * - Materialization (Data() call) is O(total_size) but cached until next edit + * - Materialization (Data() call) is O(total_size) but cached until next edit; + * a single-piece table (an unedited file) is viewed in place, not copied * - Line index is lazily rebuilt on first line-based query after edits * - Range and Find operations use lightweight caches for repeated queries * @@ -83,18 +84,39 @@ public: // Content management void Clear(); + // Replace the content with `bytes`, taken over without copying (a loaded + // file). Undo spans into the old storage are invalidated, as by Clear(). + void AdoptOriginal(std::string &&bytes); + // Accessors - char *Data() + // The whole text, contiguous; nullptr when empty. Valid until the next + // modification. + [[nodiscard]] const char *Data() const { - materialize(); - return materialized_.empty() ? nullptr : materialized_.data(); + const std::string_view v = ContentView(); + return v.empty() ? nullptr : v.data(); } - [[nodiscard]] const char *Data() const + // The whole text as one view: in place for a single piece, else the + // materialized copy (built once per modification). Valid until the next + // modification. + [[nodiscard]] std::string_view ContentView() const; + + // A view of [byte_offset, byte_offset + len), clamped to the content: in + // place when the range lies within one piece (no copy), else into the + // materialized text. Valid until the next modification. + [[nodiscard]] std::string_view View(std::size_t byte_offset, std::size_t len) const; + + // Call fn(const char *data, std::size_t len) for each piece, in order: + // the content without materializing it. + template + void ForEachChunk(Fn &&fn) const { - const_cast(this)->materialize(); - return materialized_.empty() ? nullptr : materialized_.data(); + for (const Piece &p: pieces_) { + if (p.len > 0) + fn((p.src == Source::Original ? original_ : add_).data() + p.start, p.len); + } } @@ -205,7 +227,7 @@ private: void RebuildLineIndex() const; // Underlying storages - std::string original_; // unused for builder use-case, but kept for API symmetry + std::string original_; // a loaded file's bytes (AdoptOriginal) std::string add_; std::vector pieces_; diff --git a/Swap.cc b/Swap.cc index 4386d7b..28453ef 100644 --- a/Swap.cc +++ b/Swap.cc @@ -43,24 +43,6 @@ constexpr std::uint8_t MAGIC[8] = {'K', 'T', 'E', '_', 'S', 'W', 'P', '\0'}; constexpr std::uint32_t VERSION = 2; -static std::string -snapshot_buffer_bytes(const Buffer &b) -{ - const std::size_t nrows = b.Nrows(); - std::string out; - // Cheap lower bound: sum of row sizes. - std::size_t approx = 0; - for (std::size_t i = 0; i < nrows; i++) - approx += b.GetLineView(i).size(); - out.reserve(approx); - for (std::size_t i = 0; i < nrows; i++) { - auto v = b.GetLineView(i); - out.append(v.data(), v.size()); - } - return out; -} - - static fs::path xdg_state_home() { @@ -1313,7 +1295,7 @@ SwapManager::RecordCheckpoint(Buffer &buf, const bool urgent_flush) p.buf = &buf; p.type = SwapRecType::CHKPT; p.urgent_flush = urgent_flush; - p.chkpt = snapshot_buffer_bytes(buf); + p.chkpt = buf.Bytes(); enqueue(std::move(p)); }