Skip to content

Commit

Permalink
Prepare to enable pantsd by default (#9826)
Browse files Browse the repository at this point in the history
### Problem

Pants' runtime overhead is influenced by multiple factors:
1) packaging overhead in virtualenvs and pexes
2) import time for python code
3) time to walk the filesystem and (re)fingerprint inputs
4) time to run `@rule`s with unchanged inputs

Pantsd helps to address all of these issues, and has just (re-)reached maturity.

### Solution

Prepare to enable `pantsd` by default by deprecating not setting the `enable_pantsd` flag. Also, set the flag for pantsbuild/pants.

### Result

noop time when running in a virtualenv (such as in [github.com/pantsbuild/example-python](https://github.com/pantsbuild/example-python)) drops from `~2.4s` to `~1.6s`. Followup work (moving fingerprinting to the server, porting the client to rust) is expected to push this below `500ms`.

There are a collection of known issues that might impact users tracked in https://github.com/pantsbuild/pants/projects/5, some of which we will address via dogfooding. Others are matters of expectation: "I wouldn't expect that to work".

One of the most significant issues though is #7654: we should consider making a push before `1.29.0` to remove use of global mutable state to allow for concurrent pants runs with pantsd under v2.

Fixes #4438.
  • Loading branch information
stuhood authored May 23, 2020
1 parent f35afbc commit 93a66ca
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 110 deletions.
7 changes: 7 additions & 0 deletions build-support/bin/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,18 @@ REQUIREMENTS_3RDPARTY_FILES=(
# internal backend packages and their dependencies which it doesn't have,
# and it'll fail. To solve that problem, we load the internal backend package
# dependencies into the pantsbuild.pants venv.
#
# TODO: Starting and stopping pantsd repeatedly here works fine, but because the
# created venvs are located within the buildroot, pantsd will fingerprint them on
# startup. Production usecases should generally not experience this cost, because
# either pexes or venvs (as created by the `pants` script that we distribute) are
# created outside of the buildroot.
function execute_packaged_pants_with_internal_backends() {
pip install --ignore-installed \
-r pants-plugins/3rdparty/python/requirements.txt &> /dev/null && \
pants \
--no-verify-config \
--no-enable-pantsd \
--pythonpath="['pants-plugins/src/python']" \
--backend-packages="[\
'pants.backend.codegen',\
Expand Down
1 change: 1 addition & 0 deletions pants.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ local_artifact_cache = "%(pants_bootstrapdir)s/artifact_cache"
[GLOBAL]
print_exception_stacktrace = true
v2 = false
enable_pantsd = true

# Enable our own custom loose-source plugins as well as contribs.
pythonpath = [
Expand Down
4 changes: 4 additions & 0 deletions pants.travis-ci.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ travis_parallelism = 4
# Override color support detection - we want colors and Travis supports them.
colors = true

# TODO: Eventually we would like pantsd enabled in CI as well, but we blanket disable it for
# now in order to smooth off rough edges locally.
enable_pantsd = false

[black]
args = ["--quiet"]

Expand Down
1 change: 1 addition & 0 deletions src/python/pants/bin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ python_library(
'src/python/pants/base:build_environment',
'src/python/pants/base:build_file',
'src/python/pants/base:cmd_line_spec_parser',
'src/python/pants/base:deprecated',
'src/python/pants/base:exception_sink',
'src/python/pants/base:exceptions',
'src/python/pants/base:exiter',
Expand Down
13 changes: 13 additions & 0 deletions src/python/pants/bin/pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from dataclasses import dataclass
from typing import List, Mapping

from pants.base.deprecated import deprecated_conditional
from pants.base.exception_sink import ExceptionSink
from pants.base.exiter import ExitCode
from pants.bin.remote_pants_runner import RemotePantsRunner
Expand Down Expand Up @@ -81,6 +82,18 @@ def run(self, start_time: float) -> ExitCode:
global_bootstrap_options.print_exception_stacktrace
)

# TODO: When we remove this deprecation, we'll change the default for the option to true.
deprecated_conditional(
lambda: global_bootstrap_options.is_default("enable_pantsd"),
removal_version="1.30.0.dev0",
entity_description="--enable-pantsd defaulting to False",
hint_message=(
"Pantsd improves runtime performance and will be enabled by default in the 1.30.x "
"stable releases. To prepare for that change, we recommend setting the "
"`[GLOBAL] enable_pantsd` setting to `True` in your pants.toml or pants.ini file."
),
)

if self._should_run_with_pantsd(global_bootstrap_options):
try:
return RemotePantsRunner(self.args, self.env, options_bootstrapper).run(start_time)
Expand Down
19 changes: 11 additions & 8 deletions src/python/pants/init/options_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from pants.option.global_options import GlobalOptions
from pants.subsystem.subsystem import Subsystem
from pants.util.dirutil import fast_relpath_optional
from pants.util.ordered_set import OrderedSet

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -134,13 +135,15 @@ def compute_pantsd_invalidation_globs(buildroot, bootstrap_options):
Combines --pythonpath and --pants-config-files files that are in {buildroot} dir with those
invalidation_globs provided by users.
"""
invalidation_globs = set()
globs = set(
sys.path
+ bootstrap_options.pythonpath
+ bootstrap_options.pants_config_files
+ bootstrap_options.pantsd_invalidation_globs
)
invalidation_globs = OrderedSet()
globs = [
*sys.path,
*bootstrap_options.pythonpath,
*bootstrap_options.pants_config_files,
"!*.pyc",
"!__pycache__/",
*bootstrap_options.pantsd_invalidation_globs,
]

for glob in globs:
if glob.startswith("!"):
Expand All @@ -155,7 +158,7 @@ def compute_pantsd_invalidation_globs(buildroot, bootstrap_options):
f"Changes to {glob}, outside of the buildroot, will not be invalidated."
)

return list(sorted(invalidation_globs))
return list(invalidation_globs)

@classmethod
def create(cls, options_bootstrapper, build_configuration, init_subsystems=True):
Expand Down
9 changes: 5 additions & 4 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,15 +486,16 @@ def register_bootstrap_options(cls, register):
help="Write logs to files under this directory.",
)

# This facilitates bootstrap-time configuration of pantsd usage such that we can
# determine whether or not to use the Pailgun client to invoke a given pants run
# without resorting to heavier options parsing.
register(
"--enable-pantsd",
advanced=True,
type=bool,
default=False,
help="Enables use of the pants daemon (and implicitly, the v2 engine). (Beta)",
help=(
"Enables use of the pants daemon (pantsd). pantsd can significantly improve "
"runtime performance by lowering per-run startup cost, and by caching filesystem "
"operations and @rule execution."
),
)

# Whether or not to make necessary arrangements to have concurrent runs in pants.
Expand Down
75 changes: 60 additions & 15 deletions src/python/pants/testutil/pants_run_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
import subprocess
import sys
import unittest
from contextlib import contextmanager
from contextlib import contextmanager, suppress
from dataclasses import dataclass
from operator import eq, ne
from pathlib import Path
from threading import Lock
from typing import Any, Callable, List, Optional, Union
from typing import Any, Callable, Iterator, List, Optional, Union

from colors import strip_color

Expand Down Expand Up @@ -557,42 +558,86 @@ def file_renamed(self, prefix, test_name, real_name):
os.rename(real_path, test_path)

@contextmanager
def temporary_file_content(self, path, content, binary_mode=True):
def temporary_directory_literal(self, path: Union[str, Path],) -> Iterator[None]:
"""Temporarily create the given literal directory under the buildroot.
The path being created must not already exist. Any parent directories will also be created
temporarily.
"""
path = os.path.realpath(path)
assert path.startswith(
os.path.realpath(get_buildroot())
), "cannot write paths outside of the buildroot!"
assert not os.path.exists(path), "refusing to overwrite an existing path!"

parent = os.path.dirname(path)
parent_ctx = (
suppress() if os.path.isdir(parent) else self.temporary_directory_literal(parent)
)

with parent_ctx:
try:
os.mkdir(path)
yield
finally:
os.rmdir(path)

@contextmanager
def temporary_file_content(
self, path: Union[str, Path], content, binary_mode=True
) -> Iterator[None]:
"""Temporarily write content to a file for the purpose of an integration test."""
path = os.path.realpath(path)
assert path.startswith(
os.path.realpath(get_buildroot())
), "cannot write paths outside of the buildroot!"
assert not os.path.exists(path), "refusing to overwrite an existing path!"
mode = "wb" if binary_mode else "w"
with open(path, mode) as fh:
fh.write(content)
try:
yield
finally:
os.unlink(path)

parent = os.path.dirname(path)
parent_ctx = (
suppress() if os.path.isdir(parent) else self.temporary_directory_literal(parent)
)
with parent_ctx:
try:
with open(path, mode) as fh:
fh.write(content)
yield
finally:
os.unlink(path)

@contextmanager
def with_overwritten_file_content(self, file_path, temporary_content=None):
def with_overwritten_file_content(
self,
file_path: Union[str, Path],
temporary_content: Optional[Union[bytes, str, Callable[[bytes], bytes]]] = None,
) -> Iterator[None]:
"""A helper that resets a file after the method runs.
It will read a file, save the content, maybe write temporary_content to it, yield, then write the
original content to the file.
:param file_path: Absolute path to the file to be reset after the method runs.
:param temporary_content: Optional content to write into the file.
:param temporary_content: Content to write to the file, or a function from current content
to new temporary content.
"""
with open(file_path, "r") as f:
with open(file_path, "rb") as f:
file_original_content = f.read()

try:
if temporary_content is not None:
with open(file_path, "w") as f:
f.write(temporary_content)
if callable(temporary_content):
content = temporary_content(file_original_content)
elif isinstance(temporary_content, bytes):
content = temporary_content
else:
content = temporary_content.encode()
with open(file_path, "wb") as f:
f.write(content)
yield

finally:
with open(file_path, "w") as f:
with open(file_path, "wb") as f:
f.write(file_original_content)

@contextmanager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from pants.testutil.pants_run_integration_test import PantsRunIntegrationTest
from pants.util.collections import assert_single_element
from pants.util.contextutil import temporary_dir
from pants.util.dirutil import is_executable, read_file, safe_file_dump
from pants.util.dirutil import is_executable
from pants.util.enums import match
from pants_test.backend.python.tasks.util.wheel import name_and_platform

Expand Down Expand Up @@ -129,30 +129,20 @@ def test_ctypes_binary_creation(self, toolchain_variant):

@_toolchain_variants
def test_ctypes_native_language_interop(self, toolchain_variant):
# TODO: consider making this mock_buildroot/run_pants_with_workdir into a
# PantsRunIntegrationTest method!
with self.mock_buildroot(
dirs_to_copy=[self._binary_interop_target_dir]
) as buildroot, buildroot.pushd():

# Replace strict_deps=False with nothing so we can override it (because target values for this
# option take precedence over subsystem options).
orig_wrapped_math_build = read_file(self._wrapped_math_build_file)
without_strict_deps_wrapped_math_build = re.sub(
"strict_deps=False,", "", orig_wrapped_math_build
)
safe_file_dump(self._wrapped_math_build_file, without_strict_deps_wrapped_math_build)

# Replace strict_deps=False with nothing so we can override it (because target values for this
# option take precedence over subsystem options).
with self.with_overwritten_file_content(
self._wrapped_math_build_file, lambda c: re.sub(b"strict_deps=False,", b"", c)
):
# This should fail because it does not turn on strict_deps for a target which requires it.
pants_binary_strict_deps_failure = self.run_pants_with_workdir(
pants_binary_strict_deps_failure = self.run_pants(
command=["binary", self._binary_target_with_interop],
# Explicitly set to True (although this is the default).
config={
"native-build-step": {"toolchain_variant": toolchain_variant.value},
# TODO(#6848): don't make it possible to forget to add the toolchain_variant option!
"native-build-settings": {"strict_deps": True},
},
workdir=os.path.join(buildroot.new_buildroot, ".pants.d"),
)
self.assert_failure(pants_binary_strict_deps_failure)
self.assertIn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,15 @@ def test_pydist_invalidation(self):
"""Test that the current version of a python_dist() is resolved after modifying its
sources."""
hello_run = f"{self.hello_install_requires_dir}:main_with_no_conflict"
run_target = lambda: self.run_pants(command=["--quiet", "run", hello_run])

with self.mock_buildroot(
dirs_to_copy=[self.hello_install_requires_dir]
) as buildroot, buildroot.pushd():
run_target = lambda: self.run_pants_with_workdir(
command=["--quiet", "run", hello_run],
workdir=os.path.join(buildroot.new_buildroot, ".pants.d"),
build_root=buildroot.new_buildroot,
)

unmodified_pants_run = run_target()
self.assert_success(unmodified_pants_run)
self._assert_nation_and_greeting(unmodified_pants_run.stdout_data)

# Modify one of the source files for this target so that the output is different.
py_source_file = os.path.join(self.hello_install_requires_dir, "hello_package/hello.py")
with open(py_source_file, "r") as f:
orig_contents = f.read()
# Replace hello! with hello?
modified_contents = re.sub("!", "?", orig_contents)
with open(py_source_file, "w") as f:
f.write(modified_contents)
unmodified_pants_run = run_target()
self.assert_success(unmodified_pants_run)
self._assert_nation_and_greeting(unmodified_pants_run.stdout_data)

# Modify one of the source files for this target so that the output is different.
py_source_file = os.path.join(self.hello_install_requires_dir, "hello_package/hello.py")
with self.with_overwritten_file_content(py_source_file, lambda c: re.sub(b"!", b"?", c)):
modified_pants_run = run_target()
self.assert_success(modified_pants_run)
self._assert_nation_and_greeting(modified_pants_run.stdout_data, punctuation="?")
Expand Down
Loading

0 comments on commit 93a66ca

Please sign in to comment.