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
30 changes: 19 additions & 11 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,20 @@ impl SourceContext {

// For ephemeral files (content provided), store it and create FileInformation
// For disk-backed files (no content), try to read from disk for FileInformation only
let (stored_content, content_for_info) = match content {
let (stored_content, file_info) = match content {
Some(c) => {
// Ephemeral file: store content and use it for FileInformation
(Some(c.clone()), Some(c))
// Ephemeral file: index the content, then store it (no clone)
let info = FileInformation::new(&c);
(Some(c), Some(info))
}
None => {
// Disk-backed file: don't store content, but try to read for FileInformation
(None, std::fs::read_to_string(&path).ok())
let info = std::fs::read_to_string(&path)
.ok()
.map(|c| FileInformation::new(&c));
(None, info)
}
};

let file_info = content_for_info.as_ref().map(|c| FileInformation::new(c));
self.files.push(SourceFile {
path,
content: stored_content,
Expand Down Expand Up @@ -121,13 +123,19 @@ impl SourceContext {
}

// Process content same as add_file
let (stored_content, content_for_info) = match content {
Some(c) => (Some(c.clone()), Some(c)),
None => (None, std::fs::read_to_string(&path).ok()),
let (stored_content, file_info) = match content {
Some(c) => {
let info = FileInformation::new(&c);
(Some(c), Some(info))
}
None => {
let info = std::fs::read_to_string(&path)
.ok()
.map(|c| FileInformation::new(&c));
(None, info)
}
};

let file_info = content_for_info.as_ref().map(|c| FileInformation::new(c));

// Add to files vec and create mapping
let index = self.files.len();
self.files.push(SourceFile {
Expand Down
40 changes: 36 additions & 4 deletions src/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::types::{FileId, Location};
use crate::{SourceContext, SourceInfo};
use std::borrow::Cow;

/// Result of mapping a position back to an original file
#[derive(Debug, Clone, PartialEq)]
Expand All @@ -28,10 +29,14 @@ impl SourceInfo {
// Compute the absolute offset in the file
let absolute_offset = start_offset + offset;

// Get file content: use stored content for ephemeral files, or read from disk
let content = match &file.content {
Some(c) => c.clone(),
None => std::fs::read_to_string(&file.path).ok()?,
// Get file content: borrow the stored content for ephemeral
// files, or read from disk. `offset_to_location` only needs a
// `&str`, so the in-memory case must not clone — callers map
// one offset per AST node, and a clone here made that
// O(nodes × file size) (quarto-dev/q2 bd-jn7r22g8).
let content: Cow<'_, str> = match &file.content {
Some(c) => Cow::Borrowed(c.as_str()),
None => Cow::Owned(std::fs::read_to_string(&file.path).ok()?),
};

// Convert offset to Location with row/column using efficient binary search
Expand Down Expand Up @@ -134,6 +139,33 @@ mod tests {
assert_eq!(mapped.location.column, 0);
}

#[test]
fn test_map_offset_disk_backed_file() {
// `content: None` files are read from disk on demand (the `Owned`
// arm); they must resolve exactly like in-memory files.
let dir = std::env::temp_dir().join(format!(
"quarto-source-map-map-offset-{}",
std::process::id()
));
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("disk.qmd");
std::fs::write(&path, "hello\nwörld\n").unwrap();

let mut ctx = SourceContext::new();
let file_id = ctx.add_file(path.to_string_lossy().into_owned(), None);
assert!(ctx.get_file(file_id).unwrap().content.is_none());

let info = SourceInfo::original(file_id, 0, 13);
// offset 9 is the 'r' after the two-byte 'ö': row 1, column 2 (chars)
let mapped = info.map_offset(9, &ctx).unwrap();
assert_eq!(mapped.file_id, file_id);
assert_eq!(mapped.location.offset, 9);
assert_eq!(mapped.location.row, 1);
assert_eq!(mapped.location.column, 2);

std::fs::remove_dir_all(&dir).unwrap();
}

#[test]
fn test_map_offset_substring() {
let mut ctx = SourceContext::new();
Expand Down
75 changes: 75 additions & 0 deletions tests/alloc_budget.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
//! `SourceInfo::map_offset` must not allocate proportionally to the file.
//!
//! Regression test for quarto-dev/q2's bd-jn7r22g8: through 0.1.3,
//! `map_offset` cloned the entire in-memory file content on every call so it
//! could pass a `&str` to `FileInformation::offset_to_location`. Callers that
//! map one offset per AST node (pampa's list loose/tight detection maps two
//! per list item) then paid O(nodes × file size) — 26 % of a 1.1 MB, 15k-item
//! document's render time.
//!
//! This lives in its own integration-test binary because it installs a
//! counting `#[global_allocator]`, which is per-binary.

use std::alloc::{GlobalAlloc, Layout, System};
use std::sync::atomic::{AtomicUsize, Ordering};

use quarto_source_map::{SourceContext, SourceInfo};

struct CountingAllocator;

/// Total bytes requested from the allocator since process start.
static ALLOCATED: AtomicUsize = AtomicUsize::new(0);

// SAFETY: delegates every call to `System` unchanged; only adds a counter.
unsafe impl GlobalAlloc for CountingAllocator {
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
ALLOCATED.fetch_add(layout.size(), Ordering::Relaxed);
// SAFETY: same contract as the caller's.
unsafe { System.alloc(layout) }
}

unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
// SAFETY: `ptr` came from `System.alloc` with this `layout`.
unsafe { System.dealloc(ptr, layout) }
}
}

#[global_allocator]
static GLOBAL: CountingAllocator = CountingAllocator;

/// Build a ~1 MB, many-line in-memory file.
fn one_megabyte_file() -> String {
let line = "- item with some words, `code`, and *emphasis* on it\n";
line.repeat((1 << 20) / line.len() + 1)
}

#[test]
fn map_offset_on_in_memory_file_allocates_a_bounded_amount() {
let content = one_megabyte_file();
let total = content.len();
let mut ctx = SourceContext::new();
let file_id = ctx.add_file("big.qmd".to_string(), Some(content));
let info = SourceInfo::original(file_id, 0, total);

const CALLS: usize = 1000;
let before = ALLOCATED.load(Ordering::Relaxed);
for i in 0..CALLS {
// Spread the offsets over the whole file so every call lands on a
// different line; the result is checked so the loop can't be
// optimized away.
let offset = (i * 7919) % total;
let mapped = info.map_offset(offset, &ctx).expect("offset is in bounds");
assert_eq!(mapped.location.offset, offset);
}
let allocated = ALLOCATED.load(Ordering::Relaxed) - before;

// The mapping itself allocates nothing; leave generous headroom for the
// test harness. Cloning the file once per call would be ~1 GB here.
const BUDGET: usize = 64 * 1024;
assert!(
allocated < BUDGET,
"{CALLS} map_offset calls on a {total}-byte in-memory file allocated \
{allocated} bytes (budget {BUDGET}); map_offset must borrow the \
stored content, not clone it"
);
}
Loading