significantly improve treesitter performance while editing large files

[pascalkuthe]

While working on an unrelated PR related to treesitter performance I noticed that editing text is extremely slow for large files with loads of injections.
The testcase I was looking at during testing is the largest file in the Linux kernel: drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_2_0_sh_mask.h

From the flamegraph (see below) I noticed that most of the time is actually spent inside helix comparing language layers.

After some digging I found that helix checks if an injection layer is already present (and hence can be incrementally updated) by iterating the entire list of injections. This has worst-case performance of O(N^2).
For example the Linux kernel header I was investigating contains 27859 injections (comments) all inside the top layer, so about 776 million comparisons are performed (on every edit).

This PR fixes this problem by replacing the linear search with a Hashtable that is built at the beginning of the update function.
The Hashtable allows replacing the linear search with O(1) lookups, so the time complexity is decreased to O(N).

I am using a RawTable from hashbrown here because I did not want to change the collection of layers (the closes substitute is a IndexMap, but that does not allow reusing indices like the HopeSlotMap).

After this change there is still some delay in the huge Linux kernel header while typing but now typing a single latter takes maybe 0.4s compared to 2s previously (very unscientifically benchmarked).
Looking at the flamegraph (see below) now the bottleneck is treesitter itself (which might be potentially speedup in the future by setting a byte range on the query cursor although that will be more challenging).
While the file provided here was an extreme case, this should be a win (or not matter) for all languages/files.

[the-mikedavis]

This is cool, nice work diving into this!

I'm pretty well out of my depth on the content of the changes; I left some small readability and documentation nits

[pascalkuthe]

Test failure on windows looks spurious to me. Its some problems with saving files which is not touched at all by this PR

[the-mikedavis]

Ah yep looks like a flake https://github.com/helix-editor/helix/actions/runs/3451158551/jobs/5760200072

I reran it and now it's green

[archseer]

For example the Linux kernel header I was investigating contains 27859 injections (comments) all inside the top layer, so about 776 million comparisons are performed (on every edit).

I wish we could reduce the number of empty injections. tree-sitter-comment seems like a bit of a hack and a lot of the layers will just be empty (I had to add a fast path for that during render)

[pascalkuthe]

For example the Linux kernel header I was investigating contains 27859 injections (comments) all inside the top layer, so about 776 million comparisons are performed (on every edit).

I wish we could reduce the number of empty injections. tree-sitter-comment seems like a bit of a hack and a lot of the layers will just be empty (I had to add a fast path for that during render)

I think with this PR the overhead of these empty injections is pretty small.
I tried disabling the comment injections doesn't really change the performance in any noticeable way (with this PR) even for that huge header (neither while editing or just navigating). Almost all performance overhead comes from the query cursor in the root layer being slow (both during the update and during rendering).
I agree it doesn't feel great but I can't think of a better way to highlight comments and it doesn't seem to cause any problems once this PR is merged (which will also offer improvements for other languages with many injections like markdown).

[archseer]

Why do we need to pull in RawTable? Why not just HashMap::with_capacity()?

[pascalkuthe]

The point of the RawTable is not the capacity/insert_no_grow. That just a nice bonus.

A HashMap<K,V> is essentially a RawTable<(K,V)> internally. However we want a LanguageLayer -> LayerId map here so using a normal HashMap<LanguageLayer, LayerId> would require that we clone all LanguageLayers (we can not store references into Syntax::layers because it is mutated later).
With a RawTable we can just store the LayerId as RawTable<LayerId> .
When we need access to the original keys to resolve hash collisions during lookup later we can just use the LayerId to index back into self.layers.

IndexMap does essentially the same thing internally (it just doesn't fit well here because we want to keep using a HopSlotMap whereas indexmap forces us to use essentially a Vec).

I also use the exact same strategy with RawTable for the interning in imara-diff (to avoid pulling in indexmap which offers a lot more functionality). So we will depend on hashbrown anyway (in fact we already do so as a dependy of unicode-linebreak)

[archseer]

Why not just insert()? It's guaranteed to avoid allocation if there's enough capacity, but it won't panic if that's not the case

[pascalkuthe]

I usually use insert_no_grow with RawTable if the capacity is trivially large enough for two reasons:

• It's faster: Swiss tables need to check an additional byte to determine if a bucket is empty so the overhead is larger then for a Vec because you need to read from a random location in the table. The complexity of this bounds check also means its basically never optimized out.
• insert requires a closure for rehashing existing entries in case the table needs to be resized (which is impossible because we have enough capacity). That either adds a unnecessary colsure that calculates a hash or a weird looking unrechable as shown below so I usually prefer the insert_no_grow variant over the other options:

layers_table.insert(hash, layer_id, |_| unreachable!("table should be large enough"));
layers_table.insert(hash, layer_id, |rehashed_layer| hasher.hash_one(self.layers[rehashed_layer])));

[archseer]

&self.config.language as *const _ as *const () avoids the transmute:

fn main() {
    let  a = vec![1];
    
    let ptr = &a as *const _ as *const ();
    println!("{:?}", ptr);
}

[archseer]

Though hmm, since it's a wrapper type it might not be the same thing

[pascalkuthe]

Yeah its not the same, because Grammar is a Newtype wrapper around a pointer:

struct Grammar(*const ());

You can definitely implement this with pointer cast but at that point you are re-implementing transmute_copy :

let language_ptr: *const Grammar = &self.config.language as *const Grammar;
let raw_language_ptr: *const *const () = language as *const *const ();
let language = *raw_language_ptr; 

I think this is one of those rare cases where transmute is the cleanest option. I use it only to get to the inner value of the newtype wrapper here (which is repr(transpraent))

[archseer]

Looks like this is pulling in an older version of hashbrown

[pascalkuthe]

This PR actually pulls in the newest version of hashbrown.
Both textwrap and unicode-linebreaks still depend on an old version.
imara-diff uses the newest version aswell so once the new ropey version lands and my other two PRs are merged we depend on that version anyway.
I do not want to downgrade hashbrown in imara-diff, because other crates that depend on it (gitoxide for example) already use the newest version (and in the future crates will rather upgrade then downgrade).

It's once again one of those unfortunate ecosystem splits where unicode-lines and textwrap want to maintain a MSRV of 1.56 but hashbrown had raised it MSRV to 1.61.
It's a bit unfortunate that these ecosystem splits happens but I think depending on the newest version and waiting for other dependencies to bump is the correct way to go.