Add a LRU cache to limit indexing memory usage. by voodoos · Pull Request #2079 · ocaml/merlin

voodoos · 2026-06-15T13:48:57Z

Change the way small values are handled by storing them along their parent.
Make filenames relative to the current working directory of the indexer.
Disable related-uids compression

This work is based on #2050

voodoos · 2026-06-15T14:09:08Z

cc @art-w if you want to have a quick 🍔 look ?

art-w · 2026-06-16T14:44:01Z

-    }
+let resolve_filename store ~filename =
+  if Filename.is_relative filename then
+    Filename.concat (Filename.dirname store.filename) filename


nitpick: I believe filename could contain ../../ (introduced in relativize below), which could mess up relativize when we write later? (I guess the result wouldn't be wrong but would have ../../path/../otherpath)

art-w · 2026-06-16T14:52:09Z

+         could lead to memory leaks if the user of the cache keeps a reference
+         to the cell. *)


There are some TODOs here with nice ideas :) (but not essentials)

voodoos · 2026-06-24T16:45:54Z

Thanks a lot for the review @art-w. I think I addressed all the important comments. These are probably worth a re-review 🙂

voodoos · 2026-06-30T14:57:48Z

I ran two benchmarks with a LRU large enough to store the entire date.

One that builds Merlin's @ocaml-index: in this build, there are no aggregations, only things built In_memory and written to the disk.

Command	Mean [s]	Min [s]	Max [s]
Before pointing indexes	10.570 ± 0.175	10.366	10.935
With pointing indexes	10.533 ± 0.392	10.151	11.528
With pointing + LRU	10.951 ± 0.347	10.596	11.619

One that aggregates all of Merlin indexes:

Command	Mean [ms]	Min [ms]	Max [ms]
Before pointing indexes	768.5 ± 43.1	707.9	829.1
With pointing indexes	229.4 ± 10.3	217.2	250.3
With pointing + LRU	354.9 ± 19.2	340.4	406.1

So we loose a bit of the improvements that pointing indexes brought, but we are still much faster at aggregating than before this work.

The most likely explanation for the slowdown is the move to a functional union-find, which is a necessary evil.

I think these results are quite good overall, with an aggregation that is twice as fast as before.

voodoos · 2026-06-30T15:06:39Z

And index sizes:

Version	total index size
Before pointing indexes	25M
With pointing indexes	25M
With pointing + LRU	27M

These are the index built by dune @ocaml-index so no aggregates which expect that pointing indexes has no impact.

And the size of the aggregate of all these indexes:

Version	total index size
Before pointing indexes	25M
With pointing indexes	4.9M
With pointing + LRU	4.1M

art-w

Thanks! It's looking good, I only have some minor questions/suggestions

art-w · 2026-07-02T10:15:44Z

-    index.related_uids
+    index.related_uids;
+  Uid_map.schema type_ufstore iter
+    (fun _iter _uid _content -> ())


Don't we need to schema iter on the Uid_set.t stored in _content?

Currently the Uid_set is not granular: module Uid_set = Shape.Uid.Set.
Should we make it granular ? I did that in d8aa258

It required removing some fast-paths in Union_find.union that were relying on polymorphic comparison. If it turns out to be important we can add an equal function to granular set.

art-w · 2026-07-02T10:24:07Z

+  Uid_map.fold
+    (fun uid content store ->
+      match content with
+      | Root _ -> ensure store uid


If ensure terminates early because Uid_map.mem uid s1 and there are no Link to that Root in s2, then I believe we need to compute the Uid_set.union of s2 with s1? (otherwise store will only have the s1 set?)

(or is it the case that if we only have a root-with-no-link then the uid set is the singleton {uid} which is guaranteed to be in the s1 set anyway?)

I think we don't even have "roots with no link" since we always register at least a pair of uids during the indexing...

Fixed in db3ab43

voodoos · 2026-07-03T14:37:55Z

My changes following the latest review do not have any significant performance impact.

voodoos · 2026-07-09T15:00:11Z

Ok, I think I addressed all your remaining comments @art-w !

art-w

Thanks, great work!

- Change the way small values are handled by storing them along their parent. - Make filenames relative to the current working directory of the indexer. - Disable related-uids compression Suggested-by: ArthurW <arthur@tarides.com> Co-authored-by: Lucccyo <cha.git@mailo.fr> Co-authored-by: Tim ats <tim.arnouts@protonmail.com> Co-authored-by: ArthurW <arthur@tarides.com>

…_disk_ptr`

Before we could rely on deduplicate to flag links worth caching, but now we have both the LRU which could trigger multiple reads to the same loc and the smalls pointing to parents (which are potentially not marked as Serialized_reused) Suggested-by: ArthurW <arthur@tarides.com>

This required removing a fast-path in Union_find.union that was relying on polymorphic comparison. I it turns out to be important we can add an equal function to granular set.

voodoos · 2026-07-16T06:02:03Z

Thank you for the thorough review @art-w. Merging now.

voodoos mentioned this pull request Jun 15, 2026

Add LRU to improve the memory usage of the indexer #2050

Open

voodoos added a commit to voodoos/merlin that referenced this pull request Jun 15, 2026

Add changelog for ocaml#2079

d503604

art-w reviewed Jun 16, 2026

View reviewed changes

voodoos force-pushed the lru_cache_2 branch from b910591 to 4e03ff1 Compare June 24, 2026 14:56

art-w reviewed Jul 2, 2026

View reviewed changes

voodoos force-pushed the lru_cache_2 branch from 8af7670 to d289fb2 Compare July 3, 2026 13:04

art-w approved these changes Jul 15, 2026

View reviewed changes

Lucccyo and others added 17 commits July 16, 2026 07:41

Add changelog for ocaml#2079

f20f772

Promote test change

5ae38c7

refmt

70af090

On_disk_small: Reuse store and loc from parents

88680e9

Promote parent in LRU when fetching a child

8873a25

Rename fetch_parent

6f1f871

Make schema optional in On_disk to remove placeholder usages of `On…

418a890

…_disk_ptr`

Factorize merge_union function

ba73179

Always upgrade reused links schemas

ebd1f00

Store info directly in In_cache instead of the LRU cell

3b94bad

Use kb for chache size flag, default to 1gb

5cbfbd0

Remove todo

d8c9331

Document default cache size

bc9c08e

find_and_wompress: directly return the root

ba39e1e

Union_find.union, no need for a new link

daf5b2a

voodoos added 11 commits July 16, 2026 07:41

Make the union find store more granular.

50cd87a

This required removing a fast-path in Union_find.union that was relying on polymorphic comparison. I it turns out to be important we can add an equal function to granular set.

Add missing cache use

69afef5

Factor out the cache logic

d9e1c02

Also skip smalls that have already been skipped

b80cfc7

Remove unused Serialized_reused

5a288d9

Remove remaining occurrence of Serialized_reused

7b8bf4c

Split On_disk_ptr in two to better distinguish smalls

8302914

Push missing fix

50c0ad2

Fix Union_find.merge in the general case

f5205a0

In_cache: remove redundant status field

844955d

Refactor Union_find.merge

ee42496

voodoos force-pushed the lru_cache_2 branch from 310496d to ee42496 Compare July 16, 2026 05:42

voodoos merged commit 69de75b into ocaml:main Jul 16, 2026
7 of 10 checks passed

		could lead to memory leaks if the user of the cache keeps a reference
		to the cell. *)

Uh oh!

Conversation

voodoos commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

voodoos commented Jun 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

art-w Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

art-w Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

voodoos commented Jun 24, 2026

Uh oh!

voodoos commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

voodoos commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

art-w left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

art-w Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

voodoos Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

art-w Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

voodoos Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

voodoos Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

voodoos commented Jul 3, 2026

Uh oh!

voodoos commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

art-w left a comment

Choose a reason for hiding this comment

Uh oh!

voodoos commented Jul 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

voodoos commented Jun 15, 2026 •

edited

Loading

voodoos commented Jun 30, 2026 •

edited

Loading

voodoos commented Jun 30, 2026 •

edited

Loading

voodoos Jul 3, 2026 •

edited

Loading

voodoos commented Jul 9, 2026 •

edited

Loading