Repository: bazelbuild/bazel
Issue: Memory usage regression in bazel 0.14.0 and 0.14.1
Labels: under investigation
P1

On Angular and related projects, we run builds on CI in a docker container.  We use the `--local_resources` flag to workaround
https://github.com/bazelbuild/bazel/issues/3645
which causes random failures where Bazel tries to allocate too much memory, since it asks the OS how much RAM is available rather than the containerization host.

After updating to 0.14.0 (and also observed in 0.14.1) we have the problem again:
https://github.com/angular/angular/issues/24484
it affects Angular users who send PRs which are failing on CI.

An example failure:
https://circleci.com/gh/gregmagolan/angular-bazel-example/442?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

There is not much indication what is going wrong, but it looks the same as errors we got before adding the `--local_resources` flag.

We are rolling back Bazel in Angular and related projects to 0.13.

Issue comments:
Please provide the contents of jvm.out.
> Please provide the contents of jvm.out.

I ssh'd into CircleCI when this failure occurs and checked jvm.out and it an empty file after the crash. The failure looks like this:

```
[1] Segmentation fault
[0] 
[0] Server terminated abruptly (error code: 14, error message: '', log file: '/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/server/jvm.out')
[0] 
[0] bazel run //src:prodserver exited with code 37
```

It doesn't always occur in the same place (there are a few steps in CI that I've seen it fail at) and on occasion it passes.

Is there anywhere else besides jvm.out that might give a hint on the crash?
If the file is empty then that's a sign that the gRPC client/server connection crashed. We've seen a similar case before where we didn't implement flow control correctly for file uploads to remote machines.

Do you have remote caching or execution enabled? If yes, please provide the flags you are using.
If these are the right flags, then it doesn't look like it:

```
[0] INFO: Options provided by the client:
[0]   Inherited 'common' options: --isatty=0 --terminal_columns=80

[0] INFO: Reading rc options for 'run' from /home/circleci/ng/tools/bazel.rc:
[0]   Inherited 'build' options: --strategy=TypeScriptCompile=worker --strategy=AngularTemplateCompile=worker --symlink_prefix=dist/

[0] INFO: Reading rc options for 'run' from /etc/bazel.bazelrc:
[0]   Inherited 'build' options: --noshow_progress --announce_rc --experimental_strict_action_env --experimental_repository_cache=/home/circleci/bazel_repository_cache --local_resources=3072,2.0,1.0
```
Hmm, that's odd - this seems to be a null build. Here's a more complete snippet from a passing run:
```
$ bazel build test/...
...
INFO: Analysed target //test/e2e:e2e (1 packages loaded).
INFO: Found 1 target...
Target //test/e2e:e2e up-to-date:
  dist/bin/test/e2e/app.spec.d.ts
...
$ yarn e2e-prodserver && yarn e2e-devserver
$ concurrently "bazel run //src:prodserver" "while ! nc -z 127.0.0.1 5432; do sleep 1; done && protractor" --kill-others --success first
...
[0] INFO: Analysed target //src:prodserver (1 packages loaded).
[0] INFO: Found 1 target...
[0] Target //src:prodserver up-to-date:
[0]   dist/bin/src/prodserver_bin.sh
[0]   dist/bin/src/prodserver
[0] INFO: Elapsed time: 15.070s, Critical Path: 9.76s
[0] INFO: 2 processes: 1 processwrapper-sandbox, 1 worker.
[0] INFO: Running command line: dist/bin/src/prodserver src -p 5432
...
[0] history-server listening on port 5432; Ctrl+C to stop
...
[0] bazel run //src:prodserver exited with code null
$ concurrently "bazel run //src:devserver" "while ! nc -z 127.0.0.1 5432; do sleep 1; done && protractor" --kill-others --success first
...
[0] INFO: Analysed target //src:devserver (0 packages loaded).
[0] INFO: Found 1 target...
[0] Target //src:devserver up-to-date:
[0]   dist/bin/src/devserver.MF
[0]   dist/bin/src/scripts_devserver.MF
[0]   dist/bin/src/devserver
[0] INFO: Elapsed time: 4.219s, Critical Path: 0.07s
[0] INFO: 0 processes.
[0] INFO: Running command line: dist/bin/src/devserver
[0] Server listening on http://ad2f21ad8e4a:5432/
```
The failing build fails to run //src:prodserver, but it's still just two actions.
I think the reference to #3645 is a red herring.
If I understand correctly, then you're running the containers with a 4 GB limit. --local_resources does not actually restrict Bazel's own memory usage, but maybe that's the intent?

IIRC, Bazel is set to 4 GB max memory, which is unaffected by --local_resources. Maybe Bazel is trying to allocate too much memory and the container is killing it? Technically, this doesn't imply that Bazel's memory use has actually increased - we might be using a slightly different gc configuration or allocate memory more rapidly, and that might push it over the limit.

If that's correct, then you should try adding --host_jvm_args=-Xmx2G to the Bazel invocation, like this:
```
bazel --host_jvm_args=-Xmx2G build ...
```
or like this in the bazelrc:
```
startup --host_jvm_args=-Xmx2G
```
Thanks for investigating, Ulf!

Your simple explanation is right. The build passes three times in a row with the 2G heap limit for the bazel JVM. https://circleci.com/gh/alexeagle/workflows/angular-bazel-example/tree/test-bazel-0.14.1

I also have a PR out to update Angular to use 0.14 again
https://github.com/angular/angular/pull/24512
where I've increased the size of the VM used to run one of the jobs.

This should fix it for us. I'm not sure what else we could do on this issue, other than improve the "guard rails" here so it's harder to get the wrong memory limit, or easier to debug. Feel free to close if you don't want to take further action.

Pull request review comments:
can assert_size itself be a test? then you don't need this extra rule, and dummy.sh can be hidden away.
Do nothing, feel better?
This is an interesting approach. What was the reasoning behind? assert_size is a nice Skylark rule, it only feels weird to be used like this in a BUILD file. Can you create and run an assert_size() target from the sh test?
> can assert_size itself be a test?

Yes it can! Done.

> Can you create and run an assert_size() target from the sh test?

That'd beat the purpose of analysis-time assertion and of not building the inputs. See the new comments in the docstring.
Now even less! (Removed.)
Oh, I see now! That's smart!

Patch that resolved the issue:
diff --git a/src/BUILD b/src/BUILD
index d2431844fb6fed..0d8d060237f92b 100644
--- a/src/BUILD
+++ b/src/BUILD
@@ -1,6 +1,7 @@
 # Packaging
 
 load(":embedded_tools.bzl", "srcsfile")
+load(":rule_size_test.bzl", "rule_size_test")
 
 md5_cmd = "set -e -o pipefail && cat $(SRCS) | sort | %s | awk '{ print $$1; }' > $@"
 
@@ -182,6 +183,17 @@ py_binary(
     "_with_jdk",
 ]]
 
+rule_size_test(
+    name = "embedded_tools_size_test",
+    src = ":embedded_tools_srcs",
+    # WARNING: Only adjust the number in `expect` if you are intentionally
+    # adding or removing embedded tools. Know that the more embedded tools there
+    # are in Bazel, the bigger the binary becomes and the slower Bazel starts.
+    expect = 503,
+    margin = 5,  # percentage
+    visibility = ["//visibility:private"],
+)
+
 filegroup(
     name = "embedded_jdk",
     srcs = select({
diff --git a/src/rule_size_test.bzl b/src/rule_size_test.bzl
new file mode 100644
index 00000000000000..29d0e813d26874
--- /dev/null
+++ b/src/rule_size_test.bzl
@@ -0,0 +1,130 @@
+# Copyright 2018 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Defines a test rule that asserts the number of output files in another rule.
+
+This rule operates in Bazel's analysis phase, not in its execution phase, and so
+it's faster than a conventional test rule would be.
+
+Furthermore this rule's action does not depend on any of the inputs (because the
+assertion is done in the analysis phase) so Bazel won't even build the input
+files to run the test. The test has constant execution time.
+
+=== Use ===
+
+Use this rule to assert the size of a filegroup or any other rule and catch
+sudden, unexpected changes in the size.
+
+The `margin` attribute allows specifying a tolerance value (percentage), to
+allow for organic, expected growth or shrinkage of the target rule.
+
+=== Example ===
+
+The "resources_size_test" test fails if the number of files in
+"resources" changes from 123 by more than 3 percent:
+
+    filegroup(
+        name = "resources",
+        srcs = glob(["**"]) + [
+            "//foo/bar:resources"
+            "//baz:resources",
+        ],
+    )
+
+    rule_size_test(
+        name = "resources_size_test",
+        src = ":resources",
+
+        # Expect 123 files in ":resources", with an error margin of 3% to allow
+        # for slight changes.
+        expect = 123,
+        margin = 3,
+    )
+
+"""
+
+def _impl(ctx):
+    if ctx.attr.expect < 0:
+        fail("ERROR: rule_size_test.expect must be positive")
+
+    if ctx.attr.margin < 0 or ctx.attr.margin > 100:
+        # Do not allow more than 100% change in size.
+        fail("ERROR: rule_size_test.margin must be in range [0..100]")
+
+    if ctx.attr.expect == 0 and ctx.attr.margin != 0:
+        # Allow no margin when expecting 0 files, to avoid division by zero.
+        fail("ERROR: rule_size_test.margin must be 0 when " +
+             "rule_size_test.expect is 0")
+
+    amount = len(ctx.attr.src[DefaultInfo].files)
+
+    if ctx.attr.margin > 0:
+        if amount >= ctx.attr.expect:
+            diff = amount - ctx.attr.expect
+        else:
+            diff = ctx.attr.expect - amount
+
+        if ((diff * 100) / ctx.attr.expect) > ctx.attr.margin:
+            fail(("ERROR: rule_size_test: expected %d file(s) within %d%% " +
+                  "error margin, got %d file(s) (%d%% difference)") % (
+                ctx.attr.expect,
+                ctx.attr.margin,
+                amount,
+                (diff * 100) / ctx.attr.expect,
+            ))
+    elif amount != ctx.attr.expect:
+        fail(("ERROR: rule_size_test: expected exactly %d file(s), got %d " +
+              "file(s)") % (ctx.attr.expect, amount))
+
+    if ctx.attr.is_windows:
+        test_bin = ctx.actions.declare_file(ctx.label.name + ".bat")
+        ctx.actions.write(output = test_bin, content = "", is_executable = True)
+    else:
+        test_bin = ctx.actions.declare_file(ctx.label.name + ".sh")
+        ctx.actions.write(
+            output = test_bin,
+            content = "#!/bin/sh",
+            is_executable = True,
+        )
+
+    return [DefaultInfo(executable = test_bin)]
+
+_rule_size_test = rule(
+    implementation = _impl,
+    attrs = {
+        # The target whose number of output files this rule asserts. The number
+        # of output files is the size of the target's DefaultInfo.files field.
+        "src": attr.label(allow_files = True),
+        # A non-negative integer, the expected number of files that the target
+        # in `src` outputs. If 0, then `margin` must also be 0.
+        "expect": attr.int(mandatory = True),
+        # A percentage value, in the range of [0..100]. Allows for tolerance in
+        # the difference between expected and actual number of files in `src`.
+        # If 0, then the target in `src` must output exactly `expect` many
+        # files.
+        "margin": attr.int(mandatory = True),
+        # True if running on Windows, False otherwise.
+        "is_windows": attr.bool(mandatory = True),
+    },
+    test = True,
+)
+
+def rule_size_test(name, **kwargs):
+    _rule_size_test(
+        name = name,
+        is_windows = select({
+            "@bazel_tools//src/conditions:windows": True,
+            "//conditions:default": False,
+        }),
+        **kwargs)


Test patch: