From f8d7c810452ce3a4d2e7027e38f5d00426a2a917 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Fri, 15 Sep 2017 07:52:17 -0700 Subject: [PATCH] [build] switch to 4.05.0+flambda by default Summary: With flambda (`-O3`), compilation time is ~5x slower, but the backend is ~25% faster! To mitigate the atrocious compilation times, introduce a new `opt` build mode in the jbuilder files. - build in "opt" mode by default from the toplevel (so that install scripts and external users get the fastest infer by default), in "default" mode by default from infer/src (since the latter is only called directly by infer devs, for faster builds) - `make byte` is as fast as before in any mode - `make test` will build "opt" by default, which is very slow. Solution for testing (or building the models) locally: `make BUILD_MODE=default test`. - You can even change the default locally with `export BUILD_MODE=default`. The benchmarks are to be taken with a sizable pinch of salt because I ran them only once and other stuff could be running in the background. That said, the perf win is consistent across all projects, with 15-20% win in wallclock time and around 25% win in total CPU time, ~9% win in sys time, and ~25% fewer minor allocations, and ~5-10% fewer overall allocations. This is only for the backend; the capture is by and large unaffected (either the same or a tad faster within noise range). Here are the results running on OpenSSL 1.0.2d on osx (12 cores, 32G RAM) === base infer binary: 26193088 bytes compile time: 40s capture: ```lang=text real 1m7.513s user 3m11.437s sys 0m55.236s ``` analysis: ```lang=text real 5m41.580s user 61m37.855s sys 1m12.870s ``` Memory profile: ```lang=json { ... "minor_gb": 0.1534719169139862, "promoted_gb": 0.0038930922746658325, "major_gb": 0.4546157643198967, "allocated_gb": 0.6041945889592171, "minor_collections": 78, "major_collections": 23, "compactions": 7, "top_heap_gb": 0.07388687133789062, "stack_kb": 0.3984375, "minor_heap_kb": 8192.0, ... } ``` === flambda with stock options (no `-Oclassic`, just the same flags as base) Exactly the same as base. === flambda `-O3` infer binary: 56870376 bytes (2.17x bigger) compile time: 191s (4.78x slower) capture is the same as base: ```lang=text real 1m9.203s user 3m12.242s sys 0m58.905s ``` analysis is ~20% wallclock time faster, ~25% CPU time faster: ```lang=text real 4m32.656s user 46m43.987s sys 1m2.424s ``` memory usage is a bit lower too: ```lang=json { ... "minor_gb": 0.11583046615123749, // 75% of previous "promoted_gb": 0.00363825261592865, // 93% of previous "major_gb": 0.45415670424699783, // about same "allocated_gb": 0.5663489177823067, // 94% of previous "minor_collections": 73, "major_collections": 22, "compactions": 7, "top_heap_gb": 0.07165145874023438, "stack_kb": 0.3359375, "minor_heap_kb": 8192.0, ... } ``` === flambda `-O2` Not nearly as exciting as `-O3`, but the compilation cost is still quite high: infer: 37826856 bytes compilation of infer: 100s Capture and analysis timings are mostly the same as base. Reviewed By: jberdine Differential Revision: D4867979 fbshipit-source-id: 99230b7 --- CONTRIBUTING.md | 9 +++++++-- Makefile | 25 ++++++++++++++++++++----- build-infer.sh | 3 +-- infer/src/Makefile | 1 + infer/src/atd/jbuild.in | 6 ++++-- infer/src/istd/jbuild.in | 12 +++++++----- infer/src/jbuild-workspace.in | 1 + infer/src/jbuild.common.in | 27 +++++++++++++++++++++------ infer/src/jbuild.in | 12 ++++++++---- 9 files changed, 70 insertions(+), 26 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 07b540270..44e5961a1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,11 +29,13 @@ make devsetup ### Tips and Tricks -- Build the code: `make -j`. +- Build the code faster: `make -j BUILD_MODE=default`. By default `make` builds infer with flambda + enabled, which makes it very slow (but makes infer significantly faster). - Faster edit/build cycle when working on OCaml code inside infer/src/: build inside infer/src/ (skips building the models after infer has been built), and build bytecode instead of native: - `make -j -C infer/src byte`. You need to have run `make -j` at some point before. + `make -j -C infer/src byte`. You need to have run `make -j` (with or without `BUILD_MODE=default`) + at some point before. - In general, `make` commands from the root of the repository make sure that dependencies are in a consistent and up-to-date state (e.g., they rebuild infer and the models before running steps that @@ -44,6 +46,9 @@ make devsetup before running the test, but running `make -C infer/tests/codetoanalyze/java/infer test` will just execute the test. +- To switch the default build mode to flambda disabled, you can `export BUILD_MODE=default` in your + shell. + ## Hacking on the Code in facebook-clang-plugins Infer uses `ASTExporter` from the [facebook-clang-plugins](https://github.com/facebook/facebook-clang-plugins) diff --git a/Makefile b/Makefile index 2fe453219..64743bce7 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,11 @@ default: infer ROOT_DIR = . include $(ROOT_DIR)/Makefile.config +# override this for faster builds (but slower infer) +BUILD_MODE ?= opt + +MAKE_SOURCE = $(MAKE) -C $(SRC_DIR) INFER_BUILD_DIR=_build/$(BUILD_MODE) + ifneq ($(UTOP),no) BUILD_SYSTEMS_TESTS += infertop build_infertop_print build_infertop_test: test_build @@ -147,22 +152,22 @@ fmt_all: .PHONY: src_build_common src_build_common: $(QUIET)$(call silent_on_success,Generating source dependencies,\ - $(MAKE) -C $(SRC_DIR) src_build_common) + $(MAKE_SOURCE) src_build_common) .PHONY: src_build src_build: src_build_common $(QUIET)$(call silent_on_success,Building native Infer,\ - $(MAKE) -C $(SRC_DIR) infer) + $(MAKE_SOURCE) infer) .PHONY: byte byte: src_build_common $(QUIET)$(call silent_on_success,Building byte Infer,\ - $(MAKE) -C $(SRC_DIR) byte) + $(MAKE_SOURCE) byte) .PHONY: test_build test_build: src_build_common $(QUIET)$(call silent_on_success,Testing Infer builds without warnings,\ - $(MAKE) -C $(SRC_DIR) test) + $(MAKE_SOURCE) test) ifeq ($(IS_FACEBOOK_TREE),yes) byte src_build_common src_build test_build: fb-setup @@ -608,7 +613,17 @@ devsetup: Makefile.autoconf if [ "$$infer_repo_is_in_manpath" != "0" ]; then \ printf "$(TERM_INFO) echo 'export MANPATH=\"%s/infer/man\":\$$MANPATH' >> \"$$shell_config_file\"$(TERM_RESET)\n" "$(ABSOLUTE_ROOT_DIR)" >&2; \ fi; \ - fi + fi; \ + test "$$BUILD_MODE" = "default" || \ + echo >&2; \ + echo '$(TERM_INFO)*** NOTE: Set `BUILD_MODE=default` in your shell to disable flambda by default.$(TERM_RESET)' >&2; \ + echo '$(TERM_INFO)*** NOTE: Compiling with flambda is ~5 times slower than without, so unless you are$(TERM_RESET)' >&2; \ + echo '$(TERM_INFO)*** NOTE: testing infer on a very large project it will not be worth it. Use the$(TERM_RESET)' >&2; \ + echo '$(TERM_INFO)*** NOTE: commands below to set the default build mode. You can then use `make BUILD_MODE=opt`$(TERM_RESET)' >&2; \ + echo '$(TERM_INFO)*** NOTE: when you really do want to enable flambda.$(TERM_RESET)' >&2; \ + echo >&2; \ + printf "$(TERM_INFO) export BUILD_MODE=default$(TERM_RESET)\n" >&2; \ + printf "$(TERM_INFO) echo 'export BUILD_MODE=default' >> \"$$shell_config_file\"$(TERM_RESET)\n" >&2 $(QUIET)PATH=$(ORIG_SHELL_PATH); if [ "$$(ocamlc -where 2>/dev/null)" != "$$($(OCAMLC) -where)" ]; then \ echo >&2; \ echo '$(TERM_INFO)*** NOTE: The current shell is not set up for the right opam switch.$(TERM_RESET)' >&2; \ diff --git a/build-infer.sh b/build-infer.sh index 8581b31e4..8a2097380 100755 --- a/build-infer.sh +++ b/build-infer.sh @@ -16,9 +16,8 @@ INFER_ROOT="$SCRIPT_DIR" INFER_DEPS_DIR="$INFER_ROOT/dependencies/infer-deps" PLATFORM="$(uname)" NCPU="$(getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1)" -OCAML_VERSION=${OCAML_VERSION:-"4.05.0"} +OCAML_VERSION=${OCAML_VERSION:-"4.05.0+flambda"} OPAM_LOCK_URL=${OPAM_LOCK_URL:-"https://github.com/rgrinberg/opam-lock"} - INFER_OPAM_SWITCH_DEFAULT=infer-"$OCAML_VERSION" function usage() { diff --git a/infer/src/Makefile b/infer/src/Makefile index aff0b563e..e895d0d5f 100644 --- a/infer/src/Makefile +++ b/infer/src/Makefile @@ -12,6 +12,7 @@ include $(ROOT_DIR)/Makefile.config ETC_DIR = $(INFER_DIR)/etc # paths to BUILD_DIR are relative because that's how jbuilder likes it +# can be overriden to specify another build mode (eg opt) INFER_BUILD_DIR = _build/default ATDGEN_SUFFIXES = _t.ml _t.mli _j.ml _j.mli diff --git a/infer/src/atd/jbuild.in b/infer/src/atd/jbuild.in index f640f7a59..1aa5e2314 100644 --- a/infer/src/atd/jbuild.in +++ b/infer/src/atd/jbuild.in @@ -3,12 +3,14 @@ let cflags = common_cflags @ ["-w"; "-27-32-34-35-39"] -;; Format.sprintf {| +;; Format.sprintf + {| (library ((name InferGenerated) (flags (%s)) + (ocamlopt_flags (%s)) (libraries (atdgen)) )) |} - (String.concat " " cflags) + (String.concat " " cflags) (String.concat " " common_optflags) |> Jbuild_plugin.V1.send diff --git a/infer/src/istd/jbuild.in b/infer/src/istd/jbuild.in index 4300d4497..4c140144c 100644 --- a/infer/src/istd/jbuild.in +++ b/infer/src/istd/jbuild.in @@ -1,12 +1,14 @@ -(* -*- tuareg -*- *) -(* NOTE: prepend jbuild.common to this file! *) - -;; Format.sprintf {| +;; (* -*- tuareg -*- *) + (* NOTE: prepend jbuild.common to this file! *) + Format.sprintf + {| (library ((name InferStdlib) (flags (%s)) + (ocamlopt_flags (%s)) (libraries (%s)) )) |} - (String.concat " " common_cflags) (String.concat " " common_libraries) + (String.concat " " common_cflags) (String.concat " " common_optflags) + (String.concat " " common_libraries) |> Jbuild_plugin.V1.send diff --git a/infer/src/jbuild-workspace.in b/infer/src/jbuild-workspace.in index bb5c3ebfa..a4d347424 100644 --- a/infer/src/jbuild-workspace.in +++ b/infer/src/jbuild-workspace.in @@ -6,4 +6,5 @@ ;; files. (context ((switch @OPAMSWITCH@) (name default))) +(context ((switch @OPAMSWITCH@) (name opt))) (context ((switch @OPAMSWITCH@) (name test))) diff --git a/infer/src/jbuild.common.in b/infer/src/jbuild.common.in index 2d79e59c4..f2e39c49a 100644 --- a/infer/src/jbuild.common.in +++ b/infer/src/jbuild.common.in @@ -1,6 +1,19 @@ (* -*- tuareg -*- *) (* use strings so that it looks like OCaml even before substituting, e.g. to use ocamlformat *) +type build_mode = Default | Opt | Test + +let build_mode = + match Jbuild_plugin.V1.context with + | "test" + -> Test + | "default" + -> Default + | "opt" + -> Opt + | ctx + -> invalid_arg ("unknown context: " ^ ctx) + let is_yes = String.equal "yes" let clang = is_yes "@BUILD_C_ANALYZERS@" @@ -27,13 +40,15 @@ let common_cflags = ; "-w" ; warnings ] in - match Jbuild_plugin.V1.context with - | "test" - -> "-warn-error" :: fatal_warnings :: common_flags - | "default" + match build_mode with + | Default | Opt -> common_flags - | ctx - -> invalid_arg ("unknown context: " ^ ctx) + | Test + -> "-warn-error" :: fatal_warnings :: common_flags + +let common_optflags = match build_mode with + | Opt -> ["-O3"] + | Default | Test -> [] let common_libraries = (if java then ["javalib"; "ptrees"; "sawja"] else []) diff --git a/infer/src/jbuild.in b/infer/src/jbuild.in index f24cc418d..2191ef2d9 100644 --- a/infer/src/jbuild.in +++ b/infer/src/jbuild.in @@ -74,35 +74,39 @@ let stanzas = (library ((name InferModules) (flags (%s)) + (ocamlopt_flags (%s)) (libraries (%s)) (modules (:standard \ %s infertop)) (preprocess (pps (ppx_compare ppx_sexp_conv -no-check))) )) |} - (String.concat " " infer_cflags) (String.concat " " infer_libraries) - (String.concat " " infer_binaries) + (String.concat " " infer_cflags) (String.concat " " common_optflags) + (String.concat " " infer_libraries) (String.concat " " infer_binaries) ; Format.sprintf {| (executables ((names (%s)) (flags (%s -open InferModules)) + (ocamlopt_flags (%s)) (libraries (InferModules)) (modules (%s)) (preprocess (pps (ppx_compare ppx_sexp_conv -no-check))) )) |} (String.concat " " infer_binaries) (String.concat " " infer_cflags) - (String.concat " " infer_binaries) + (String.concat " " common_optflags) (String.concat " " infer_binaries) ; Format.sprintf {| (executable ((name infertop) (flags (%s)) + (ocamlopt_flags (%s)) (libraries (utop InferModules)) (modules (:standard \ %s)) (link_flags (-linkall -warn-error -31)))) |} - (String.concat " " infer_cflags) (String.concat " " infer_binaries) ] + (String.concat " " infer_cflags) (String.concat " " common_optflags) + (String.concat " " infer_binaries) ] @ List.map (fun source -> Printf.sprintf "(rule (%s %s %s))" (copy_action_of_source source) source