Perf: update the line index in place; renderers read only visible rows
- PieceTable::Insert/Delete adjust a valid line index directly (shift later line starts, add or drop starts for inserted or deleted newlines) instead of invalidating it, which forced a byte-by-byte rescan of the whole buffer on the next line query. On a 38 MB, 1M-line buffer, insert + line query drops from 14.7 ms to 0.28 ms. - The terminal and ImGui renderers fetch only the rows they draw (Nrows + GetLineString) instead of Buffer::Rows(), which rebuilt a string for every line of the file on the first frame after each edit. Qt's line-count uses switch to Nrows(). ImGuiRenderer.cc was syntax-checked with g++ and clang++ against the vendored ImGui headers; the GUI targets could not be built here (no SDL2 or Qt6). Audit findings P1, P2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
+12
-10
@@ -66,7 +66,9 @@ ImGuiRenderer::Draw(Editor &ed)
|
|||||||
if (!buf) {
|
if (!buf) {
|
||||||
ImGui::TextUnformatted("[no buffer]");
|
ImGui::TextUnformatted("[no buffer]");
|
||||||
} else {
|
} else {
|
||||||
const auto &lines = buf->Rows();
|
// Per-row access only: Rows() rebuilds a string for every line of the
|
||||||
|
// file after each edit, which made every frame O(file size).
|
||||||
|
const std::size_t nlines = buf->Nrows();
|
||||||
std::size_t cy = buf->Cury();
|
std::size_t cy = buf->Cury();
|
||||||
std::size_t cx = buf->Curx();
|
std::size_t cx = buf->Curx();
|
||||||
const float line_h = ImGui::GetTextLineHeight();
|
const float line_h = ImGui::GetTextLineHeight();
|
||||||
@@ -175,7 +177,7 @@ ImGuiRenderer::Draw(Editor &ed)
|
|||||||
// Compute the visible row range and skip rendering work for off-screen
|
// Compute the visible row range and skip rendering work for off-screen
|
||||||
// lines. ImGui clips drawing, but string allocation, tab expansion,
|
// lines. ImGui clips drawing, but string allocation, tab expansion,
|
||||||
// syntax-highlight lookups, and width measurement are not free.
|
// syntax-highlight lookups, and width measurement are not free.
|
||||||
const std::size_t total_rows = lines.size();
|
const std::size_t total_rows = nlines;
|
||||||
std::size_t first_vis = 0;
|
std::size_t first_vis = 0;
|
||||||
std::size_t last_vis = total_rows; // exclusive
|
std::size_t last_vis = total_rows; // exclusive
|
||||||
if (row_h > 0.0f && total_rows > 0) {
|
if (row_h > 0.0f && total_rows > 0) {
|
||||||
@@ -221,14 +223,14 @@ ImGuiRenderer::Draw(Editor &ed)
|
|||||||
if (by_l < 0)
|
if (by_l < 0)
|
||||||
by_l = 0;
|
by_l = 0;
|
||||||
std::size_t by = static_cast<std::size_t>(by_l);
|
std::size_t by = static_cast<std::size_t>(by_l);
|
||||||
if (by >= lines.size())
|
if (by >= nlines)
|
||||||
by = lines.empty() ? 0 : (lines.size() - 1);
|
by = (nlines == 0) ? 0 : (nlines - 1);
|
||||||
|
|
||||||
if (lines.empty())
|
if (nlines == 0)
|
||||||
return {0, 0};
|
return {0, 0};
|
||||||
|
|
||||||
// Expand tabs for the clicked line
|
// Expand tabs for the clicked line
|
||||||
std::string line_clicked = static_cast<std::string>(lines[by]);
|
std::string line_clicked = buf->GetLineString(by);
|
||||||
const std::size_t tabw = 8;
|
const std::size_t tabw = 8;
|
||||||
std::string click_expanded;
|
std::string click_expanded;
|
||||||
click_expanded.reserve(line_clicked.size() + 16);
|
click_expanded.reserve(line_clicked.size() + 16);
|
||||||
@@ -322,7 +324,7 @@ ImGuiRenderer::Draw(Editor &ed)
|
|||||||
for (std::size_t i = first_vis; i < last_vis; ++i) {
|
for (std::size_t i = first_vis; i < last_vis; ++i) {
|
||||||
// Capture the screen position before drawing the line
|
// Capture the screen position before drawing the line
|
||||||
ImVec2 line_pos = ImGui::GetCursorScreenPos();
|
ImVec2 line_pos = ImGui::GetCursorScreenPos();
|
||||||
std::string line = static_cast<std::string>(lines[i]);
|
std::string line = buf->GetLineString(i);
|
||||||
|
|
||||||
// Expand tabs to spaces with width=8
|
// Expand tabs to spaces with width=8
|
||||||
const std::size_t tabw = 8;
|
const std::size_t tabw = 8;
|
||||||
@@ -607,8 +609,8 @@ ImGuiRenderer::Draw(Editor &ed)
|
|||||||
|
|
||||||
// Horizontal scroll: ensure cursor is visible (pixel-based for proportional fonts)
|
// Horizontal scroll: ensure cursor is visible (pixel-based for proportional fonts)
|
||||||
float cursor_px_abs = 0.0f;
|
float cursor_px_abs = 0.0f;
|
||||||
if (cy < lines.size()) {
|
if (cy < nlines) {
|
||||||
std::string cur_line = static_cast<std::string>(lines[cy]);
|
std::string cur_line = buf->GetLineString(cy);
|
||||||
const std::size_t tabw = 8;
|
const std::size_t tabw = 8;
|
||||||
// Expand tabs for cursor line to measure pixel position
|
// Expand tabs for cursor line to measure pixel position
|
||||||
std::string cur_expanded;
|
std::string cur_expanded;
|
||||||
@@ -791,7 +793,7 @@ ImGuiRenderer::Draw(Editor &ed)
|
|||||||
left += " *";
|
left += " *";
|
||||||
// Append total line count as "<n>L"
|
// Append total line count as "<n>L"
|
||||||
{
|
{
|
||||||
unsigned long lcount = static_cast<unsigned long>(buf->Rows().size());
|
unsigned long lcount = static_cast<unsigned long>(buf->Nrows());
|
||||||
left += " ";
|
left += " ";
|
||||||
left += std::to_string(lcount);
|
left += std::to_string(lcount);
|
||||||
left += "L";
|
left += "L";
|
||||||
|
|||||||
+41
-4
@@ -1,4 +1,5 @@
|
|||||||
#include <algorithm>
|
#include <algorithm>
|
||||||
|
#include <cstring>
|
||||||
#include <utility>
|
#include <utility>
|
||||||
#include <limits>
|
#include <limits>
|
||||||
#include <ostream>
|
#include <ostream>
|
||||||
@@ -358,6 +359,42 @@ PieceTable::InvalidateLineIndex() const
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
void
|
||||||
|
PieceTable::lineIndexOnInsert(std::size_t offset, const char *text, std::size_t len) const
|
||||||
|
{
|
||||||
|
std::lock_guard<std::mutex> lock(mutex_);
|
||||||
|
if (line_index_dirty_)
|
||||||
|
return;
|
||||||
|
// Line starts after the insertion point move right; a start exactly at
|
||||||
|
// the insertion point stays (the text joins that line).
|
||||||
|
auto first_after = std::upper_bound(line_index_.begin(), line_index_.end(), offset);
|
||||||
|
for (auto it = first_after; it != line_index_.end(); ++it)
|
||||||
|
*it += len;
|
||||||
|
std::vector<std::size_t> added;
|
||||||
|
const char *end = text + len;
|
||||||
|
for (const char *p = text; (p = static_cast<const char *>(std::memchr(p, '\n', end - p))) != nullptr; ++p)
|
||||||
|
added.push_back(offset + static_cast<std::size_t>(p - text) + 1);
|
||||||
|
if (!added.empty())
|
||||||
|
line_index_.insert(first_after, added.begin(), added.end());
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
void
|
||||||
|
PieceTable::lineIndexOnDelete(std::size_t offset, std::size_t len) const
|
||||||
|
{
|
||||||
|
std::lock_guard<std::mutex> lock(mutex_);
|
||||||
|
if (line_index_dirty_)
|
||||||
|
return;
|
||||||
|
// Starts in (offset, offset+len] followed a deleted newline: drop them.
|
||||||
|
// Later starts move left.
|
||||||
|
auto lo = std::upper_bound(line_index_.begin(), line_index_.end(), offset);
|
||||||
|
auto hi = std::upper_bound(lo, line_index_.end(), offset + len);
|
||||||
|
for (auto it = hi; it != line_index_.end(); ++it)
|
||||||
|
*it -= len;
|
||||||
|
line_index_.erase(lo, hi);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
void
|
void
|
||||||
PieceTable::RebuildLineIndex() const
|
PieceTable::RebuildLineIndex() const
|
||||||
{
|
{
|
||||||
@@ -405,7 +442,7 @@ PieceTable::Insert(std::size_t byte_offset, const char *text, std::size_t len)
|
|||||||
pieces_.push_back(Piece{Source::Add, add_start, len});
|
pieces_.push_back(Piece{Source::Add, add_start, len});
|
||||||
total_size_ += len;
|
total_size_ += len;
|
||||||
dirty_ = true;
|
dirty_ = true;
|
||||||
InvalidateLineIndex();
|
lineIndexOnInsert(byte_offset, add_.data() + add_start, len);
|
||||||
maybeConsolidate();
|
maybeConsolidate();
|
||||||
version_++;
|
version_++;
|
||||||
range_cache_ = {};
|
range_cache_ = {};
|
||||||
@@ -419,7 +456,7 @@ PieceTable::Insert(std::size_t byte_offset, const char *text, std::size_t len)
|
|||||||
pieces_.push_back(Piece{Source::Add, add_start, len});
|
pieces_.push_back(Piece{Source::Add, add_start, len});
|
||||||
total_size_ += len;
|
total_size_ += len;
|
||||||
dirty_ = true;
|
dirty_ = true;
|
||||||
InvalidateLineIndex();
|
lineIndexOnInsert(byte_offset, add_.data() + add_start, len);
|
||||||
coalesceNeighbors(pieces_.size() - 1);
|
coalesceNeighbors(pieces_.size() - 1);
|
||||||
maybeConsolidate();
|
maybeConsolidate();
|
||||||
version_++;
|
version_++;
|
||||||
@@ -447,7 +484,7 @@ PieceTable::Insert(std::size_t byte_offset, const char *text, std::size_t len)
|
|||||||
|
|
||||||
total_size_ += len;
|
total_size_ += len;
|
||||||
dirty_ = true;
|
dirty_ = true;
|
||||||
InvalidateLineIndex();
|
lineIndexOnInsert(byte_offset, add_.data() + add_start, len);
|
||||||
// Try coalescing around the inserted position (the inserted piece is at idx + (inner>0 ? 1 : 0))
|
// Try coalescing around the inserted position (the inserted piece is at idx + (inner>0 ? 1 : 0))
|
||||||
std::size_t ins_index = idx + (inner > 0 ? 1 : 0);
|
std::size_t ins_index = idx + (inner > 0 ? 1 : 0);
|
||||||
coalesceNeighbors(ins_index);
|
coalesceNeighbors(ins_index);
|
||||||
@@ -517,7 +554,7 @@ PieceTable::Delete(std::size_t byte_offset, std::size_t len)
|
|||||||
|
|
||||||
total_size_ -= len;
|
total_size_ -= len;
|
||||||
dirty_ = true;
|
dirty_ = true;
|
||||||
InvalidateLineIndex();
|
lineIndexOnDelete(byte_offset, len);
|
||||||
if (idx < pieces_.size())
|
if (idx < pieces_.size())
|
||||||
coalesceNeighbors(idx);
|
coalesceNeighbors(idx);
|
||||||
if (idx > 0)
|
if (idx > 0)
|
||||||
|
|||||||
@@ -175,6 +175,12 @@ private:
|
|||||||
// Line index support (rebuilt lazily on demand)
|
// Line index support (rebuilt lazily on demand)
|
||||||
void InvalidateLineIndex() const;
|
void InvalidateLineIndex() const;
|
||||||
|
|
||||||
|
// Keep a valid line index in step with an edit instead of rescanning the
|
||||||
|
// whole buffer on the next query. No-ops if the index is already dirty.
|
||||||
|
void lineIndexOnInsert(std::size_t offset, const char *text, std::size_t len) const;
|
||||||
|
|
||||||
|
void lineIndexOnDelete(std::size_t offset, std::size_t len) const;
|
||||||
|
|
||||||
void RebuildLineIndex() const;
|
void RebuildLineIndex() const;
|
||||||
|
|
||||||
// Underlying storages
|
// Underlying storages
|
||||||
|
|||||||
+2
-2
@@ -474,7 +474,7 @@ protected:
|
|||||||
left += QStringLiteral(" *");
|
left += QStringLiteral(" *");
|
||||||
|
|
||||||
// total lines suffix " <n>L"
|
// total lines suffix " <n>L"
|
||||||
unsigned long lcount = static_cast<unsigned long>(buf->Rows().size());
|
unsigned long lcount = static_cast<unsigned long>(buf->Nrows());
|
||||||
left += QStringLiteral(" ");
|
left += QStringLiteral(" ");
|
||||||
left += QString::number(static_cast<qlonglong>(lcount));
|
left += QString::number(static_cast<qlonglong>(lcount));
|
||||||
left += QStringLiteral("L");
|
left += QStringLiteral("L");
|
||||||
@@ -617,7 +617,7 @@ protected:
|
|||||||
long nr = static_cast<long>(new_rowoffs) + d_rows;
|
long nr = static_cast<long>(new_rowoffs) + d_rows;
|
||||||
if (nr < 0)
|
if (nr < 0)
|
||||||
nr = 0;
|
nr = 0;
|
||||||
const auto nrows = static_cast<long>(buf->Rows().size());
|
const auto nrows = static_cast<long>(buf->Nrows());
|
||||||
if (nr > std::max(0L, nrows - 1))
|
if (nr > std::max(0L, nrows - 1))
|
||||||
nr = std::max(0L, nrows - 1);
|
nr = std::max(0L, nrows - 1);
|
||||||
new_rowoffs = static_cast<std::size_t>(nr);
|
new_rowoffs = static_cast<std::size_t>(nr);
|
||||||
|
|||||||
+10
-8
@@ -42,7 +42,9 @@ TerminalRenderer::Draw(Editor &ed)
|
|||||||
|
|
||||||
int saved_cur_y = -1, saved_cur_x = -1; // logical cursor position within content area
|
int saved_cur_y = -1, saved_cur_x = -1; // logical cursor position within content area
|
||||||
if (buf) {
|
if (buf) {
|
||||||
const auto &lines = buf->Rows();
|
// Per-row access only: Rows() rebuilds a string for every line of the
|
||||||
|
// file after each edit, which made every frame O(file size).
|
||||||
|
const std::size_t nlines = buf->Nrows();
|
||||||
std::size_t rowoffs = buf->Rowoffs();
|
std::size_t rowoffs = buf->Rowoffs();
|
||||||
std::size_t coloffs = buf->Coloffs();
|
std::size_t coloffs = buf->Coloffs();
|
||||||
|
|
||||||
@@ -62,8 +64,8 @@ TerminalRenderer::Draw(Editor &ed)
|
|||||||
// Compute matches for this line if search highlighting is active
|
// Compute matches for this line if search highlighting is active
|
||||||
bool search_mode = ed.SearchActive() && !ed.SearchQuery().empty();
|
bool search_mode = ed.SearchActive() && !ed.SearchQuery().empty();
|
||||||
std::vector<std::pair<std::size_t, std::size_t> > ranges; // [start, end)
|
std::vector<std::pair<std::size_t, std::size_t> > ranges; // [start, end)
|
||||||
if (search_mode && li < lines.size()) {
|
if (search_mode && li < nlines) {
|
||||||
std::string sline = static_cast<std::string>(lines[li]);
|
std::string sline = buf->GetLineString(li);
|
||||||
// If regex search prompt is active (RegexSearch or RegexReplaceFind), use regex to compute highlight ranges
|
// If regex search prompt is active (RegexSearch or RegexReplaceFind), use regex to compute highlight ranges
|
||||||
if (ed.PromptActive() && (
|
if (ed.PromptActive() && (
|
||||||
ed.CurrentPromptKind() == Editor::PromptKind::RegexSearch || ed.
|
ed.CurrentPromptKind() == Editor::PromptKind::RegexSearch || ed.
|
||||||
@@ -140,8 +142,8 @@ TerminalRenderer::Draw(Editor &ed)
|
|||||||
return true;
|
return true;
|
||||||
};
|
};
|
||||||
int written = 0;
|
int written = 0;
|
||||||
if (li < lines.size()) {
|
if (li < nlines) {
|
||||||
std::string line = static_cast<std::string>(lines[li]);
|
std::string line = buf->GetLineString(li);
|
||||||
const bool vsel_on_line = vsel_active && li >= vsel_sy && li <= vsel_ey;
|
const bool vsel_on_line = vsel_active && li >= vsel_sy && li <= vsel_ey;
|
||||||
const std::size_t vsel_spot_src = vsel_on_line
|
const std::size_t vsel_spot_src = vsel_on_line
|
||||||
? std::min(buf->Curx(), line.size())
|
? std::min(buf->Curx(), line.size())
|
||||||
@@ -389,8 +391,8 @@ TerminalRenderer::Draw(Editor &ed)
|
|||||||
std::size_t cx = buf->Curx();
|
std::size_t cx = buf->Curx();
|
||||||
int cur_y = static_cast<int>(cy) - static_cast<int>(buf->Rowoffs());
|
int cur_y = static_cast<int>(cy) - static_cast<int>(buf->Rowoffs());
|
||||||
std::size_t rx_recomputed = 0;
|
std::size_t rx_recomputed = 0;
|
||||||
if (cy < lines.size()) {
|
if (cy < nlines) {
|
||||||
const std::string line_for_cursor = static_cast<std::string>(lines[cy]);
|
const std::string line_for_cursor = buf->GetLineString(cy);
|
||||||
std::size_t src_i_cur = 0;
|
std::size_t src_i_cur = 0;
|
||||||
std::size_t render_col_cur = 0;
|
std::size_t render_col_cur = 0;
|
||||||
while (src_i_cur < line_for_cursor.size() && src_i_cur < cx) {
|
while (src_i_cur < line_for_cursor.size() && src_i_cur < cx) {
|
||||||
@@ -541,7 +543,7 @@ TerminalRenderer::Draw(Editor &ed)
|
|||||||
left += " [RO]";
|
left += " [RO]";
|
||||||
// Append total line count as "<n>L"
|
// Append total line count as "<n>L"
|
||||||
if (b) {
|
if (b) {
|
||||||
unsigned long lcount = static_cast<unsigned long>(b->Rows().size());
|
unsigned long lcount = static_cast<unsigned long>(b->Nrows());
|
||||||
left += " ";
|
left += " ";
|
||||||
left += std::to_string(lcount);
|
left += std::to_string(lcount);
|
||||||
left += "L";
|
left += "L";
|
||||||
|
|||||||
@@ -11,6 +11,8 @@
|
|||||||
#include "UndoTree.h"
|
#include "UndoTree.h"
|
||||||
|
|
||||||
#include <chrono>
|
#include <chrono>
|
||||||
|
#include <random>
|
||||||
|
#include <vector>
|
||||||
#include <clocale>
|
#include <clocale>
|
||||||
#include <memory>
|
#include <memory>
|
||||||
#include <string>
|
#include <string>
|
||||||
@@ -440,3 +442,42 @@ TEST(Audit_B6_HorizontalScroll_CountsCells)
|
|||||||
// 200 cells wide on an 80-column screen: the cursor cell is 200.
|
// 200 cells wide on an 80-column screen: the cursor cell is 200.
|
||||||
ASSERT_EQ(coloffs, (std::size_t) (200 - 80 + 1));
|
ASSERT_EQ(coloffs, (std::size_t) (200 - 80 + 1));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
// P2: the line index is updated in place by Insert/Delete. Query it after
|
||||||
|
// every edit (so it is never dirty) and compare with a plain string model.
|
||||||
|
TEST(Audit_P2_IncrementalLineIndex_MatchesModel)
|
||||||
|
{
|
||||||
|
std::mt19937 rng(20260923u);
|
||||||
|
PieceTable t;
|
||||||
|
std::string model;
|
||||||
|
const char alphabet[] = "ab\nc\n\nd";
|
||||||
|
for (int step = 0; step < 3000; ++step) {
|
||||||
|
const bool do_insert = model.empty() || (rng() % 3 != 0);
|
||||||
|
if (do_insert) {
|
||||||
|
const std::size_t off = model.empty() ? 0 : rng() % (model.size() + 1);
|
||||||
|
std::string text;
|
||||||
|
const std::size_t n = 1 + rng() % 6;
|
||||||
|
for (std::size_t i = 0; i < n; ++i)
|
||||||
|
text.push_back(alphabet[rng() % (sizeof(alphabet) - 1)]);
|
||||||
|
t.Insert(off, text.data(), text.size());
|
||||||
|
model.insert(off, text);
|
||||||
|
} else {
|
||||||
|
const std::size_t off = rng() % model.size();
|
||||||
|
const std::size_t n = 1 + rng() % 5;
|
||||||
|
t.Delete(off, n);
|
||||||
|
model.erase(off, n);
|
||||||
|
}
|
||||||
|
// Reference line starts.
|
||||||
|
std::vector<std::size_t> starts{0};
|
||||||
|
for (std::size_t i = 0; i < model.size(); ++i)
|
||||||
|
if (model[i] == '\n')
|
||||||
|
starts.push_back(i + 1);
|
||||||
|
ASSERT_EQ(t.LineCount(), starts.size());
|
||||||
|
for (std::size_t l = 0; l < starts.size(); ++l) {
|
||||||
|
const auto [s, e] = t.GetLineRange(l);
|
||||||
|
ASSERT_EQ(s, starts[l]);
|
||||||
|
ASSERT_EQ(e, (l + 1 < starts.size()) ? starts[l + 1] : model.size());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user