Collect rules with Appendable_list.t instead of Id.Map.t

[pmwhite]

Based on #9050, the main goal is to replace the unioning of a bunch of maps with what I expect to be a cheaper operation. I've benchmarked a similar patch on Jane Street's code base and saw enough improvement to think that this is a worthwhile refactor. My thinking was to have review done first here on GitHub; once it is able to be merged I'll import it, run benchmark it again and report back with a summary of the results.

[pmwhite]

It's probably not worth reviewing until #9050 gets merged, so that the diff in this PR isn't as large. I wish GitHub knew how to handle PR chains.

[pmwhite]

Converting to draft because I was silly and forgot to run tests, which are broken.

[Alizter]

The bench shows some very promising results!

Metric	Last PR value	Last main value	Delta
[Clean Sandbox] Build Time	299.965	316.930	-5.35289%
[Clean] Build Time	298.799	310.847	-3.87600%
[Null Sandbox] Build Time	3.58720	3.63885	-1.41939%
[Null] Build Time	3.57499	3.66382	-2.42454%

[Alizter]

However it seems there are times when the union is eliminating duplicates:

/odoc-wrapped-lib.t/run.t
   $ dune build @doc
+  Error: Multiple rules generated for
+  _build/default/_doc/_html/l/index.html.json:
+  - <internal location>
+  - <internal location>
+  -> required by alias doc
+  [1]

I wonder if these can be resolved by not duplicating them to begin with. 🤔

[pmwhite]

@Alizter Awesome, thanks for the benchmark results! Regarding the test failures, some of them are just re-orderings of rules, which is expected.

Other failures, like the one you highlighted, replace one error message with another. I suspect that the duplicate rules error message isn't new, but has just been hidden by the first error message - the re-ordering of rules changed which one actually gets displayed. Notice that the error you pointed out is very similar to #1645 , which is yet to be fixed.

[rgrinberg]

@pmwhite please re-open once you make some progress on this