Skip to content

Commit

Permalink
[make] s/ocamlbuild/jbuilder/g
Browse files Browse the repository at this point in the history
Summary:
Use jbuilder to build infer instead of ocamlbuild. This is mainly to get faster builds:

```
times in 10ms, ±differences measured in speedups, 4 cores
|                                   | ocb total |   jb | ±total | ocb user |   jb | ±user | ocb cpu |  jb | ±cpu | ocb sys |   jb | ±sys |
|-----------------------------------+-----------+------+--------+----------+------+-------+---------+-----+------+---------+------+------|
| byte from scratch                 |      6428 | 2456 |   2.62 |     7743 | 6662 |  1.16 |     138 | 331 | 2.40 |    1184 | 1477 | 0.80 |
| native from scratch               |      9841 | 4289 |   2.29 |     9530 | 8834 |  1.08 |     110 | 245 | 2.23 |    1373 | 1712 | 0.80 |
| byte after native                 |     29578 | 1602 |  18.46 |     4514 | 4640 |  0.97 |     170 | 325 | 1.91 |     543 |  576 | 0.94 |
| change infer.ml byte              |       344 |  282 |   1.22 |      292 |  215 |  1.36 |      96 |  99 | 1.03 |     040 |  066 | 0.61 |
| change infer.ml native            |       837 |  223 |   3.75 |      789 |  174 |  4.53 |      98 |  99 | 1.01 |     036 |   47 | 0.77 |
| change Config.ml byte             |       451 |  339 |   1.33 |      382 |  336 |  1.14 |      97 | 122 | 1.26 |     056 |   80 | 0.70 |
| change Config.ml native           |      4024 | 1760 |   2.29 |     4585 | 4225 |  1.09 |     127 | 276 | 2.17 |     559 |  644 | 0.87 |
| change cFrontend_config.ml byte   |       348 |  643 |   0.54 |      297 |  330 |  0.90 |      96 |  67 | 0.70 |     038 |  102 | 0.37 |
| change cFrontend_config.ml native |      1480 |  584 |   2.53 |     1435 |  906 |  1.58 |     106 | 185 | 1.75 |     136 |  178 | 0.76 |
#+TBLFM: $4=$2/$3;f2::$7=$5/$6;f2::$10=$9/$8;f2::$13=$11/$12;f2

50 cores
|                     | ocb total |   jb | ±total | ocb user |   jb | ±user | ocb cpu | jb | ±cpu | ocb sys |   jb | ±sys |
|---------------------+-----------+------+--------+----------+------+-------+---------+----+------+---------+------+------|
| byte from scratch   |      9114 | 2061 |   4.42 |     9334 | 5133 |  1.82 |         |    | 0/0  |    2566 | 1726 | 1.49 |
| native from scratch |     13481 | 3967 |   3.40 |    12291 | 7608 |  1.62 |         |    | 0/0  |    3003 | 2100 | 1.43 |
| byte after native   |      3467 | 1476 |   2.35 |     5067 | 3912 |  1.30 |         |    | 0/0  |     971 |  801 | 1.21 |
#+TBLFM: $4=$2/$3;f2::$7=$5/$6;f2::$10=$9/$8;f2::$13=$11/$12;f2
```

Menu:

1. Write a jbuild file, autogenerated from jbuild.in because we need to fill in
  some information at build-time (really, at configure time, but TODO), such as
  whether or not clang is enabled.
2. Nuke lots of stuff from infer/src/Makefile that is now in the jbuild file
3. The jbuild file lives in infer/src/ so it can see all the sources. If we put it somewhere else, eg, infer/, then `jbuilder` scans too many files (all irrelevant) and takes 2.5s to start instead of .8s. Adding irrelevant directories to jbuild-ignore does not help.
4. jbuilder does not support subdirectories, so resort to listing all the
  source files in the generated jbuild (only source directories need to be
  manually listed in jbuild.in though). Still, the generated .merlin is wrong
  and makes merlin find source files in _build, so manually tune it to get
  good merlin support. We also lose some of merlin for unit tests as it
  cannot see their build artefacts anymore.
5. checkCopyright gets its own jbuild because it's standalone. Also, remove
  some deprecation warnings in checkCopyright due to the new version of Core from
  a while ago.
6. Drop less-used Makefile features (they had regressed anyway) such as
  building individual modules. Also, building mod_dep.pdf now takes all the
  source files available so they better build (before, it would only take the
  source files from the config, eg with or without clang) (that's pretty minor).
7. The toplevel is now built as a custom toplevel because that was easier. It
  should soon be even easier: ocaml/dune#210
8. Move BUILTINS.mli to BUILTINS.ml because jbuilder is not happy about
  interface files without implementations.

In particular, I did not try to migrate too much of the Makefile logic to jbuilder,
more can be done in the future.

Reviewed By: jberdine

Differential Revision: D5573661

fbshipit-source-id: 4ca6d8f
  • Loading branch information
jvillard authored and facebook-github-bot committed Aug 10, 2017
1 parent 61d6baa commit 8de2b88
Show file tree
Hide file tree
Showing 23 changed files with 455 additions and 241 deletions.
11 changes: 8 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
# generated by build and tests
/_build
/_build_logs
/infer/_build
/infer/tests/codetoanalyze/java/*/codetoanalyze
/dependencies/infer-deps-*
_build_infer
Expand Down Expand Up @@ -93,8 +92,8 @@ buck-out/

#other
/infer/bin/infer
/infer/bin/infer.byte
/infer/bin/infer.native
/infer/bin/infer.bc
/infer/bin/infer.exe
/infer/bin/infer-analyze
/infer/bin/infer-capture
/infer/bin/infer-compile
Expand Down Expand Up @@ -151,3 +150,9 @@ infer/src/.project

# generated by Maven
/infer/annotations/target

# jbuilder
/infer/src/_build
/infer/src/jbuild
/infer/src/jbuild-workspace
/infer/src/scripts/.merlin
44 changes: 24 additions & 20 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ default: infer
ROOT_DIR = .
include $(ROOT_DIR)/Makefile.config

ifneq ($(UTOP),no)
BUILD_SYSTEMS_TESTS += infertop
build_infertop_print build_infertop_replace build_infertop_test: toplevel_test
endif

ifeq ($(BUILD_C_ANALYZERS),yes)
BUILD_SYSTEMS_TESTS += \
assembly \
Expand Down Expand Up @@ -81,8 +86,8 @@ endif
ifneq ($(BUCK),no)
BUILD_SYSTEMS_TESTS += buck genrule
# do not run these two tests in parallel otherwise Buck has a bad time
build_genrule_test: build_buck_test
build_genrule_print: build_buck_print
build_genrule_test: | build_buck_test
build_genrule_print: | build_buck_print
endif
ifneq ($(MVN),no)
BUILD_SYSTEMS_TESTS += mvn
Expand Down Expand Up @@ -149,14 +154,9 @@ byte: src_build_common
test_build: src_build_common
$(QUIET)$(call silent_on_success,Testing Infer builds without warnings,\
$(MAKE) -C $(SRC_DIR) TEST=1 byte_no_install)
# byte_no_install builds most of what toplevel needs, so it's more efficient to run the
# toplevel build straight after it rather than in parallel. Note that both targets build files
# that the other doesn't.
$(QUIET)$(call silent_on_success,Testing Infer toplevel builds,\
$(MAKE) -C $(SRC_DIR) TEST=1 toplevel)

ifeq ($(IS_FACEBOOK_TREE),yes)
byte src_build test_build: fb-setup
byte src_build_common src_build test_build: fb-setup
endif

ifeq ($(BUILD_C_ANALYZERS),yes)
Expand Down Expand Up @@ -219,7 +219,7 @@ endif
.PHONY: ocaml_unit_test
ocaml_unit_test: test_build
$(QUIET)$(call silent_on_success,Running OCaml unit tests,\
$(BUILD_DIR)/test/infer/unit/inferunit.byte)
$(BUILD_DIR)/test/inferunit.bc)

define silence_make
($(1) 2> >(grep -v "warning: \(ignoring old\|overriding\) \(commands\|recipe\) for target") \
Expand Down Expand Up @@ -296,15 +296,17 @@ check_missing_mli:
$(QUIET)for x in $$(find $(INFER_DIR)/src -name "*.ml"); do \
test -f "$$x"i || echo Missing "$$x"i; done

.PHONY: toplevel
toplevel: clang_plugin src_build_common
$(QUIET)$(MAKE) -C $(SRC_DIR) toplevel
# depend on test_build because jbuilder doesn't like running concurrently with itself
.PHONY: toplevel_test
toplevel_test: src_build_common | test_build
# build with TEST=1 as the infer_repl scripts expects to find the toplevel in the test build
$(QUIET)$(call silent_on_success,Building infer toplevel (test mode),\
$(MAKE) -C $(SRC_DIR) TEST=1 toplevel)

.PHONY: inferScriptMode_test
inferScriptMode_test: test_build
$(QUIET)$(call silent_on_success,Testing infer OCaml REPL,\
INFER_REPL_BINARY=ocaml TOPLEVEL_DIR=$(BUILD_DIR)/test/infer $(SCRIPT_DIR)/infer_repl \
$(INFER_DIR)/tests/repl/infer_batch_script.mltop)
.PHONY: toplevel
toplevel: src_build_common
$(QUIET)$(call silent_on_success,Building infer toplevel,\
$(MAKE) -C $(SRC_DIR) toplevel)

.PHONY: checkCopyright
checkCopyright:
Expand Down Expand Up @@ -342,8 +344,7 @@ else
endif

.PHONY: config_tests
config_tests: test_build ocaml_unit_test endtoend_test inferScriptMode_test \
checkCopyright validate-skel
config_tests: test_build ocaml_unit_test endtoend_test checkCopyright validate-skel
$(QUIET)$(call silent_on_success,Building Infer source dependency graph,\
$(MAKE) -C $(SRC_DIR) mod_dep.dot)

Expand Down Expand Up @@ -550,7 +551,10 @@ opam.lock: opam
$(QUIET)$(call silent_on_success,generating opam.lock,\
$(OPAM) lock --pkg $(INFER_PKG_OPAMLOCK) > opam.lock)

OPAM_DEV_DEPS = ocp-indent merlin tuareg
# This is a magical version number that doesn't reinstall the world when added on top of what we
# have in opam.lock. To upgrade this version number, manually try to install several utop versions
# until you find one that doesn't recompile the world. TODO(t20828442): get rid of magic
OPAM_DEV_DEPS = ocp-indent merlin tuareg utop.2.0.0

.PHONY: devsetup
devsetup: Makefile.autoconf
Expand Down
1 change: 1 addition & 0 deletions Makefile.autoconf.in
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ prefix = @prefix@
PYTHON_lxml = @PYTHON_lxml@
SHASUM = @SHASUM@
USER_JAVA_HOME = @USER_JAVA_HOME@
UTOP = @UTOP@
XCODE_SELECT = @XCODE_SELECT@
XCPRETTY = @XCPRETTY@

Expand Down
2 changes: 1 addition & 1 deletion Makefile.config
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ JAVA_BUILTINS_DIR = $(MODELS_DIR)/java/builtins
JAVA_MODELS_DIR = $(MODELS_DIR)/java/src
SRC_DIR = $(INFER_DIR)/src

BUILD_DIR = $(INFER_DIR)/_build
BUILD_DIR = $(SRC_DIR)/_build

JAVA_LIB_DIR = $(LIB_DIR)/java
SPECS_LIB_DIR = $(LIB_DIR)/specs
Expand Down
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ AC_ASSERT_OCAML_PKG([biniou])
AC_ASSERT_OCAML_PKG([camlzip], [zip])
AC_ASSERT_OCAML_PKG([easy-format])
AC_ASSERT_OCAML_PKG([oUnit], [], [2.0.0])
AC_CHECK_TOOL([UTOP], [utop], [no])
AC_ASSERT_OCAML_PKG([yojson])

AC_ARG_VAR([CAML_LD_LIBRARY_PATH],
Expand Down
20 changes: 0 additions & 20 deletions infer/.merlin

This file was deleted.

38 changes: 38 additions & 0 deletions infer/src/.merlin
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
B _build/default/**
PKG ANSITerminal
PKG atdgen
PKG cmdliner
PKG core
PKG javalib
PKG oUnit
PKG parmap
PKG ppx_compare
PKG ptrees
PKG sawja
PKG str
PKG unix
PKG xmlm
PKG yojson
PKG zip
FLG -principal -safe-string -short-paths -strict-formats -strict-sequence
FLG -w +a-4-9-40-41-42-45-48
FLG -open InferBaseStdlib -open InferGenerated -open InferModules
S backend
S base
S bufferoverrun
S checkers
S clang
S clang_plugin
S clang_stubs
S eradicate
S facebook
S harness
S integration
S IR
S java
S java_stubs
S labs
S opensource
S quandary
S scripts
S unit
File renamed without changes.
Loading

0 comments on commit 8de2b88

Please sign in to comment.