Shapes: bugfix and performance improvement

[trefis]

The first commit in this PR fixes the excessive memory usage reported in #10779. Following that fix we decided to build irmin (and mesure the time it takes), which appears to be a heavy user of functors. A characteristic which our previous test codebase was lacking, allowing the issue to remain undetected.

Thus we discovered a pretty big regression in compilation time on projects which make aggressive uses of functors, as well as bigger shape size than we'd previously seen.
Apart from the fix mentioned above, this PR includes a few commits aimed at mitigating those:

• 5f77c3e is an effort to maximize the sharing of shapes in Includemod by returning the input shape when it is not modified in any way.
• d0ca6b8 changes the way shapes are computed for functor applications. Previously for functors with a shape of the form Abs (X, res_shape), we would substitute the shape of the argument for X in res_shape. For functors with N-parameters, their applications implied going over the result shape N-times. With this commit we first collect all the arguments, and then go over the result shape only once.
• 39dbf66 is similar to 5f77c3e: when computing the shape of a functor application, we make sure to return the input shape when it didn't depend on the parameter.

Some measurements are available below to see the impact of those patches.

N.B. an alternative solution was suggested by @gasche: to not reduce shapes in the compiler, leave it all to merlin. This does fix the compilation time increases, as well the growth in cmt size. However, with a straightforward (some could say naïve) reduction strategy as is currently implemented in the compiler (and exposed by compiler-libs), this implies recomputing the same reduction over and over again, taking several minutes (if not hours) to fully reduce the shape of Irmin.

This could obviously be mitigated with some amount of memoization (though we haven't tried), but overall it doesn't seem like it'd be much simpler than the patches proposed here. Furthermore, I personally would rather spend a few more seconds compiling my project, than have merlin hang for a second or two when I ask it something.

And now, for some numbers:

Irmin

(This is a build of https://github.com/mirage/irmin/, which includes several packages)

Compilation time

dune build @install -j 1 , 10 runs

Branch	Mean [s]	Min [s]	Max [s]
4.14 without shapes	44.326 ± 0.294	43.892	44.715
4.14 with shapes + bugfix	68.810 ± 0.278	68.450	69.283
... + sharing in includemod	68.423 ± 0.449	67.860	69.293
... + optimize n-ary applications	50.460 ± 0.176	50.159	50.646
... + sharing of unchanged shapes	48.549 ± 0.368	48.100	49.437

Note: the noticeable performance degradation only impacts a few calls to the compiler:

	without shapes	shapes+bugfix	shapes + opts and sharing
src/irmin/.../irmin.cmo	0.399 (±0.003)	9.654 (±0.077)	2.147 (±0.038)
src/irmin/.../irmin.cmx	0.426 (±0.003)	8.317 (±0.110)	1.310 (±0.024)
src/irmin-git/.../irmin_git.cmo	1.312 (±0.037)	4.324 (±0.002)	1.654 (±0.051)
src/irmin-git/.../irmin_git.cmx	1.208 (±0.080)	3.755 (±0.001)	1.480 (±0.031)
src/irmin-mirage/git/.../irmin_mirage_git.cmo	1.038 (±0.006)	1.451 (±0.000)	1.117 (±0.032)
src/irmin-mirage/git/.../irmin_mirage_git.cmx	0.983 (±0.033)	1.358 (±0.059)	1.053 (±0.006)

For all the other invocations of the compiler (of which there are 361), the change in compilation time is less than 50ms, and often lost in the noise.
(We can make the raw logs available if people are curious)

Cmt sizes:

Relatedly, there is a noticeable increase in .cmt size on Irmin, but once again very localized:

Branch	Size occupied by all the cmt files	Size of irmin.cmt
4.14 without shapes	65M	2.5M
4.14 with shapes + bugfix	199M	102M
4.14 with shapes + opts and sharing	159M	91M

Monorepo

To not be entirely biaised towards functors, we have also reproduced the measurements on a more diverse "monorepo", which includes the following packages: containers, base, react, astring, cmdliner, cppo, dune, fpath, result, fmt, logs, re, ppxlib, ctypes and all their dependencies.

Timings

dune build @install -j 1 , 10 runs

Branch	Mean [s]	Min [s]	Max [s]
4.14 without shapes	106.531 ± 0.442	105.589	107.158
4.14 with shapes + bugfix	108.775 ± 0.686	107.915	109.985
... + sharing in includemod	108.686 ± 0.723	107.671	110.317
... + optimize n-ary applications	108.924 ± 0.613	108.115	109.943
... + sharing of unchanged shapes	109.175 ± 0.675	107.780	110.025

Cmt sizes:

Branch	Size
414-no-shapes	84M
414-shape-bugfix	85M
414-shape-nary-shared	85M

[gasche]

Can you provide repro steps to build irmin on a 4.14 switch? I just tried and it complains that ppxlib cannot be installed.

[kit-ty-kate]

@gasche using https://github.com/kit-ty-kate/opam-alpha-repository should do:

opam remote add alpha git+https://github.com/kit-ty-kate/opam-alpha-repository

[voodoos]

Can you provide repro steps to build irmin on a 4.14 switch? I just tried and it complains that ppxlib cannot be installed.

And also we did our testing on the latest 2.9.0 tag.

[Octachron]

I think it would make sense to move this code to the Shape module in order to keep the Shape.desc type morally abstract.

[Octachron]

I would rather deduplicate this optimisation, in order to ensure that the two instances are kept in sync in the future.

[gasche]

cross-ref: the original Shapes PR is #10718.

[Octachron]

Closed in favor of #10825.