Skip to content

Use ocaml{c,opt}.opt when available to build ocamldoc#8839

Closed
gasche wants to merge 3 commits intoocaml:trunkfrom
gasche:ocamldoc-ocamlc.opt
Closed

Use ocaml{c,opt}.opt when available to build ocamldoc#8839
gasche wants to merge 3 commits intoocaml:trunkfrom
gasche:ocamldoc-ocamlc.opt

Conversation

@gasche
Copy link
Copy Markdown
Member

@gasche gasche commented Jul 28, 2019

This PR uses ocaml{c,opt}.opt (when available) instead of the bytecode versions to build ocamldoc -- as mentioned in #8835.

On my machine, a sequential build of ocamldoc goes from 31s to 11s, and a parallel build (-j5) goes from 16s to 6s.

It would of course be interesting to generalize this approach to other tools than ocamldoc (everything that can be delayed to after ocaml{c,opt}.opt in the opt.opt Makefile target; I'm not sure exactly what but at least the debugger could benefit), which would suggest factorizing the conditional logic in a root auxiliary Makefile file. I decided to start only with only ocamldoc (whose build is relatively separated from the others) in this first PR, and factorize later if the logic gets used in more than one place.

gasche added 2 commits July 28, 2019 10:27
On my machine, a sequential build of (make all allopt) goes from 31s
to 11s, and a parallel build (-j5) goes from 16s to 6s.

Note however that currently the 'all' phase of ocamldoc is built by
the root Makefile before the .opt build tools are available (in the
'world' stage), so the root Makefile needs to be tweaked to realize
the full time savings: only the 'allopt' phase currently benefits
(sequential time goes from 19s to 7s).
@dra27
Copy link
Copy Markdown
Member

dra27 commented Jul 28, 2019

My overall comment in #8837 (review) still applies, but that doesn’t preclude optimising these things in advance. There’s not much beyond ocamltest and otherlibs which would benefit from this I think.

This might change some of the dependency stuff for incremental builds, I’m not sure - but it looks correct to me for the cold build, certainly.

@gasche
Copy link
Copy Markdown
Member Author

gasche commented Jul 28, 2019

I have started working on a more general PR that also changes ocamltest, debugger and otherlibs, and I find that it's in fact easier to look at all the changes at once (especially at the level of the root Makefile), so I will close this PR -- hoping to send the more general one shortly.

@gasche
Copy link
Copy Markdown
Member Author

gasche commented Jul 28, 2019

The new PR is #8840, it changes otherlibs, ocamltest, ocamldoc and the debugger. The root-Makefile diff is pleasantly simple:

 opt.opt: checknative
        $(MAKE) checkstack
        $(MAKE) runtime
        $(MAKE) core
        $(MAKE) ocaml
        $(MAKE) opt-core
        $(MAKE) ocamlc.opt
-       $(MAKE) otherlibraries $(WITH_DEBUGGER) $(WITH_OCAMLDOC) ocamltest
        $(MAKE) ocamlopt.opt
+       $(MAKE) otherlibraries $(WITH_DEBUGGER) $(WITH_OCAMLDOC) ocamltest
        $(MAKE) otherlibrariesopt
        $(MAKE) ocamllex.opt ocamltoolsopt ocamltoolsopt.opt $(OCAMLDOC_OPT) \
          ocamltest.opt

I think that this is clearly correct from a dependency perspective: we are only moving ocamlopt.opt before tools that could already be built without it.

@gasche
Copy link
Copy Markdown
Member Author

gasche commented Jul 29, 2019

(Thinking about this more, even that change should not be necessary, given that those targets should not use ocamlopt at all.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants