Bug 1776522 Comment 3 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I'm finishing this up now.  A notable change I had to make was to introduce a new "jumpref"/"jumpref-extra" pair using the crossref format.

My first attempt at this was to have output-file just directly perform crossref lookups whenever it found a symbol in a file that it had not seen yet.  The crossref entry would then be passed through a transformation function (`convert_crossref_value_to_sym_info_rep`) and saved into the map which will be written into the HTML file.  (And that symbol would never be looked up again for the file being formatted, but would be for other files in that run.)

This ended up insanely regressing performance.  Like, for dom/indexedDB/ActorsParent.cpp (a very large C++ file) there was (using only 1 data point for each measurement) a 218x regression on the formatting stage of output-file.  The move to jumpref netted us a speed-up, taking 98.9% of the current trunk run.  But again, these are individual data points.  A better metric is that output.sh now seems to take about 30 seconds longer, up to 6m31s from 5m56s, so this potentially makes us slower, but not by much.  This is an acceptable performance regression for our purposes, especially because in bug 1781179 :botond wants to do some semantic highlighting which needs something like a crossref lookup.  And even if we do move to using tree-sitter as a tokenizer in bug 1796870 which potentially would let us get syntax highlighting from just the parse tree without needing the clang details, there are things like inlay hints about the types being used that do in fact want crossref-like lookup[1].
I'm finishing this up now after having landed a bunch of important precursor work like the settings page which in turn wanted more rust-based templating, etc..  A notable change I had to make was to introduce a new "jumpref"/"jumpref-extra" pair using the crossref format.

My first attempt at this was to have output-file just directly perform crossref lookups whenever it found a symbol in a file that it had not seen yet.  The crossref entry would then be passed through a transformation function (`convert_crossref_value_to_sym_info_rep`) and saved into the map which will be written into the HTML file.  (And that symbol would never be looked up again for the file being formatted, but would be for other files in that run.)

This ended up insanely regressing performance.  Like, for dom/indexedDB/ActorsParent.cpp (a very large C++ file) there was (using only 1 data point for each measurement) a 218x regression on the formatting stage of output-file.  The move to jumpref netted us a speed-up, taking 98.9% of the current trunk run.  But again, these are individual data points.  A better metric is that output.sh now seems to take about 30 seconds longer, up to 6m31s from 5m56s, so this potentially makes us slower, but not by much.  This is an acceptable performance regression for our purposes, especially because in bug 1781179 :botond wants to do some semantic highlighting which needs something like a crossref lookup.  And even if we do move to using tree-sitter as a tokenizer in bug 1796870 which potentially would let us get syntax highlighting from just the parse tree without needing the clang details, there are things like inlay hints about the types being used that do in fact want crossref-like lookup[1].
I'm finishing this up now after having landed a bunch of important precursor work like the settings page which in turn wanted more rust-based templating, etc..  A notable change I had to make was to introduce a new "jumpref"/"jumpref-extra" pair using the crossref format.

My first attempt at this was to have output-file just directly perform crossref lookups whenever it found a symbol in a file that it had not seen yet.  The crossref entry would then be passed through a transformation function (`convert_crossref_value_to_sym_info_rep`) and saved into the map which will be written into the HTML file.  (And that symbol would never be looked up again for the file being formatted, but would be for other files in that run.)

This ended up insanely regressing performance.  Like, for dom/indexedDB/ActorsParent.cpp (a very large C++ file) there was (using only 1 data point for each measurement) a 218x regression on the formatting stage of output-file.  So I modified crossref.rs to also run the transformation function on the crossref data as we wrote it to the crossref file, building a parallel jumpref fiile.  Here's what the file sizes end up looking like:

```
ubuntu@ip-IP:~/index/mozilla-central$ du -csh *ref*
2.3G    crossref
4.3G    crossref-extra
927M    jumpref
317M    jumpref-extra
7.8G    total
```

The reason the jumpref files aren't more trivial in size is that the contents are:
- sym: The symbol!
- pretty: The pretty identifier for the symbol.
- jumps: A set of up to 3 locations for idl/def/decl (small)
- meta: The structured information payload for the symbol plus derived information.  This can get large for symbols like (pretty) `mozilla::ipc::IProtocol::AllManagedActors` where there are a ton of overrides, or `nsISupports` where there are a ton of subclasses.  For now I'm going to err on the side of keeping this data since it potentially allows for a bunch of client-side prototyping and maybe even it makes sense for a bunch of widgets or functionality to run client-side without having to talk to the server[1].  The change doesn't actually bloat the rendered HTML (with the baked-in JSON data as JS), with dom/indexedDB/ActorsParent.cpp.gz going from 1.4M on trunk to 1.3M with the patch.  This makes sense because as comment 0 points out, the ANALYSIS_DATA array was incredibly redundant and was O(symbol-referencing tokens) whereas the new SYM_INFO is O(symbols referenced in the source).

There are also situations where there are just very long symbol names that can push us over the threshold into "-extra" gifted to us by rlbox in particular.  Like:
```
w2c_std____2__pair_std____2____tree_iterator_std____2____value_type_unsigned_int__unsigned_int___std____2____tree_node_std____2____value_type_unsigned_int__unsigned_int___void_____long___bool__std____2____tree_std____2____value_type_unsigned_int__unsigned_int___std____2____map_value_compare_unsigned_int__std____2____value_type_unsigned_int__unsigned_int___std____2__less_unsigned_int___true___std____2__allocator_std____2____value_type_unsigned_int__unsigned_int_______emplace_unique_key_args_unsigned_int__std____2__piecewise_construct_t_const___std____2__tuple_unsigned_int_const____std____2__tuple____unsigned_int_const___std____2__piecewise_construct_t_const___std____2__tuple_unsigned_int_const______std____2__tuple____
```

The move to jumpref netted us a speed-up, taking 98.9% of the current trunk run.  But again, these are individual data points.  A better metric is that output.sh now seems to take about 30 seconds longer, up to 6m31s from 5m56s, so this potentially makes us slower, but not by much.  This is an acceptable performance regression for our purposes, especially because in bug 1781179 :botond wants to do some semantic highlighting which needs something like a crossref lookup.  And even if we do move to using tree-sitter as a tokenizer in bug 1796870 which potentially would let us get syntax highlighting from just the parse tree without needing the clang details, there are things like inlay hints about the types being used that do in fact want crossref-like lookup.

1:  In general my plan has been to bias towards to try and do everything server-side, even the widgets, because we want to avoid ending up in a situation where searchfox loads 10 MiB of JS at startup and you end up looking at serious latency to get results.  Having searchfox do things server-side in a way that can be cached (and pre-cached) has a lot of upsides.  (Even in cases where a user is network limited, the fancy branch prototype showed that spidering symbols via separate requests would be the real latency problem.)  And if the canonical way to do things is in rust, then we run into consistency problems if we also have a JS implementation, etc. etc.  But perhaps we can strike a balance with widgets.  

Note that the choice of the liquid templating engine was so that we could use it both on the client and the server, but I wasn't envisioning using it for something so complex.  It's definitely been an improvement on doing all the HTML generation in rust, but the ergonomics aren't good enough for something more generic.  Probably better to use an embedded JS runtime.  Although maybe if we come up with some good web components with rules that keep their behavior simple and fast, the answer can just be client-side web components.

Back to Bug 1776522 Comment 3