Skip to content

Commit

Permalink
further improve text object movement performance
Browse files Browse the repository at this point in the history
  • Loading branch information
pascalkuthe committed Nov 11, 2022
1 parent dac3df4 commit 96f2917
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
20 changes: 13 additions & 7 deletions helix-core/src/movement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,13 @@ pub fn goto_treesitter_object(

let cap_name = |t: TextObject| format!("{}.{}", object_name, t);
let mut cursor = QueryCursor::new();
let nodes = lang_config.textobject_query()?.capture_nodes_any(
let range = match dir {
Direction::Forward => (byte_pos + 1)..slice.len_bytes(),
Direction::Backward => 0..byte_pos,
};
// only search within the relevant range
cursor.set_byte_range(range);
let mut nodes = lang_config.textobject_query()?.capture_nodes_any(
&[
&cap_name(TextObject::Movement),
&cap_name(TextObject::Around),
Expand All @@ -417,12 +423,12 @@ pub fn goto_treesitter_object(
)?;

let node = match dir {
Direction::Forward => nodes
.filter(|n| n.start_byte() > byte_pos)
.min_by_key(|n| n.start_byte())?,
Direction::Backward => nodes
.filter(|n| n.end_byte() < byte_pos)
.max_by_key(|n| n.end_byte())?,
// catpure_nodes_any always returns nodes in the correct order
// so we don't need to iterate trough all of them which helps performance
// tramendously
Direction::Forward => nodes.next()?,
// captures can not be iterated in reverse so we are stuck with this
Direction::Backward => nodes.last()?,
};

let len = slice.len_bytes();
Expand Down
19 changes: 11 additions & 8 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,19 +356,22 @@ impl<'a> CapturedNode<'a> {

/// The number of matches a TS cursor can at once to avoid performance problems for medium to large files.
/// Set with `set_match_limit`.
/// Using such a limit means that we loose valid captures in so there is fundementally a tradeoff here.
/// Using such a limit means that we loose valid captures in, so there is fundamentally a tradeoff here.
///
///
/// Old tree sitter versions used a limit of 32 by default until this limit was removed in version `0.19.5` (must now best set manually).
/// However this causes performance issues for medium to large files.
/// In helix this problem caused textobjt motions to take multiple seconds to complete in medium sized rust files (3k loc).
/// However, this causes performance issues for medium to large files.
/// In helix, this problem caused treesitter motions to take multiple seconds to complete in medium-sized rust files (3k loc).
/// Neovim also encountered this problem and reintroduced this limit after it was removed upstream
/// (see https://github.com/neovim/neovim/issues/14897 and https://github.com/neovim/neovim/pull/14915).
/// The number used here is fundementally a tradeoff between breaking some obscure edgecases and performance.
/// The number used here is fundamentally a tradeoff between breaking some obscure edge cases and performance.
///
///
/// 64 was choosen because neovim uses that value.
/// It has been choosen somewhat arbitrarly (https://github.com/neovim/neovim/pull/18397) mostly based
/// upon how many issues occur in practice.
/// So this could be changed if we ran into problems.
/// A value of 64 was chosen because neovim uses that value.
/// Neovim chose this value somewhat arbitrarily (https://github.com/neovim/neovim/pull/18397) adjusting it whenever issues occur in practice.
/// However this value has been in use for a long time and due to the large userbase of neovim it is probably a good choice.
/// If this limit causes problems for a grammar in the future, it could be increased.
const TREE_SITTER_MATCH_LIMIT: u32 = 64;

impl TextObjectQuery {
Expand Down

0 comments on commit 96f2917

Please sign in to comment.