Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: memoize Rope::from_str(source_text) #5500

Closed
Boshen opened this issue Sep 5, 2024 · 2 comments · Fixed by #5518
Closed

perf: memoize Rope::from_str(source_text) #5500

Boshen opened this issue Sep 5, 2024 · 2 comments · Fixed by #5518
Assignees
Labels
A-transformer Area - Transformer / Transpiler C-bug Category - Bug C-performance Category - Solution not expected to change functional behavior, only performance

Comments

@Boshen
Copy link
Member

Boshen commented Sep 5, 2024

/// Get line and column from offset and source text.
///
/// Line number starts at 1.
/// Column number is in UTF-16 characters, and starts at 1.
///
/// This matches Babel's output.
pub fn get_line_column(offset: u32, source_text: &str) -> (usize, usize) {
let offset = offset as usize;
let rope = Rope::from_str(source_text);
// Get line number and byte offset of start of line
let line_index = rope.byte_to_line(offset);
let line_offset = rope.line_to_byte(line_index);
// Get column number
let column_index = source_text[line_offset..offset].encode_utf16().count();
// line and column are zero-indexed, but we want 1-indexed
(line_index + 1, column_index + 1)
}

fn offset_to_position(offset: usize, source_text: &str) -> Option<Position> {
let rope = Rope::from_str(source_text);
let line = rope.try_byte_to_line(offset).ok()?;
let first_char_of_line = rope.try_line_to_char(line).ok()?;
// Original offset is byte, but Rope uses char offset
let offset = rope.try_byte_to_char(offset).ok()?;
let column = offset - first_char_of_line;
Some(Position::new(line as u32, column as u32))
}

Rope::from_str(source_text) is too expensive.

image

@Boshen Boshen added C-bug Category - Bug A-transformer Area - Transformer / Transpiler labels Sep 5, 2024
@Boshen Boshen self-assigned this Sep 5, 2024
@Boshen Boshen added the C-performance Category - Solution not expected to change functional behavior, only performance label Sep 5, 2024
@IWANABETHATGUY
Copy link
Contributor

IWANABETHATGUY commented Sep 5, 2024

You could just use lineArray, if you don't need incremental remove and add a string.

@IWANABETHATGUY
Copy link
Contributor

This remind me a blog long time ago, https://code.visualstudio.com/blogs/2018/03/23/text-buffer-reimplementation

Boshen pushed a commit that referenced this issue Sep 6, 2024
Currently only used in `jsx-source`, so memorize rope in JSXSource enough for now.

close: #5500
@Dunqing Dunqing closed this as completed Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-transformer Area - Transformer / Transpiler C-bug Category - Bug C-performance Category - Solution not expected to change functional behavior, only performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants