sureshjoshi commented 3 months ago

Two problems I run into are:

As soon as I pull a new repo, coursier runs and starts downloading a bunch of stuff - even though I've never touched our java/jvm/scala/kotlin code
- This is made worse, as I have network blocking on - and courier doesn't timeout - just tries to spin forever
Slow cycle time for negligible changes

As an example of the 2nd.

After fixing a typo in a function comment, and formatting, I spend 31 seconds waiting for that to finish.

time pants fmt src/python/pants/goal/migrate_call_by_name*  
0.58s user 0.06s system 2% cpu 30.932 total

In some more egregious cases, I've waiting up to 60 seconds for a fmt/lint to run, and most of the time happens in the "scheduler" (handwaving everything before running the goal as "scheduler").

I narrowed the bulk of the time spent to commenting out #"internal_plugins.test_lockfile_fixtures", in the pants.toml - and then went further with removing the JVM entirely.

The following diff brought my cycle time down to closer to 6-7 seconds (which I think can be even better). My integration tests also run about 1 second faster, for the 1 test I run.

My git diff

```bash diff --git a/pants.toml b/pants.toml index 6b91bb1bbd..e52c003510 100644 --- a/pants.toml +++ b/pants.toml @@ -7,7 +7,6 @@ backend_packages.add = [ "pants.backend.build_files.fix.deprecations", "pants.backend.build_files.fmt.black", "pants.backend.python", - "pants.backend.experimental.python.packaging.pyoxidizer", "pants.backend.python.lint.autoflake", "pants.backend.python.lint.black", "pants.backend.python.lint.docformatter", @@ -19,28 +18,28 @@ backend_packages.add = [ "pants.backend.shell", "pants.backend.shell.lint.shellcheck", "pants.backend.shell.lint.shfmt", - "pants.backend.docker", - "pants.backend.docker.lint.hadolint", - "pants.backend.experimental.adhoc", - "pants.backend.experimental.go", - "pants.backend.experimental.java", - "pants.backend.experimental.java.lint.google_java_format", - "pants.backend.experimental.java.debug_goals", + #"pants.backend.docker", + #"pants.backend.docker.lint.hadolint", + #"pants.backend.experimental.adhoc", + #"pants.backend.experimental.go", + #"pants.backend.experimental.java", + #"pants.backend.experimental.java.lint.google_java_format", + #"pants.backend.experimental.java.debug_goals", "pants.backend.experimental.javascript", "pants.backend.experimental.javascript.lint.prettier", "pants.backend.experimental.python", - "pants.backend.experimental.python.packaging.pyoxidizer", - "pants.backend.experimental.scala", - "pants.backend.experimental.scala.lint.scalafmt", - "pants.backend.experimental.scala.lint.scalafix", - "pants.backend.experimental.scala.debug_goals", - "pants.backend.experimental.tools.workunit_logger", - "pants.backend.experimental.visibility", - "pants.backend.tools.preamble", - "pants.backend.tools.taplo", - "pants_explorer.server", - "internal_plugins.releases", - "internal_plugins.test_lockfile_fixtures", + #"pants.backend.experimental.python.packaging.pyoxidizer", + #"pants.backend.experimental.scala", + #"pants.backend.experimental.scala.lint.scalafmt", + #"pants.backend.experimental.scala.lint.scalafix", + #"pants.backend.experimental.scala.debug_goals", + #"pants.backend.experimental.tools.workunit_logger", + #"pants.backend.experimental.visibility", + #"pants.backend.tools.preamble", + #"pants.backend.tools.taplo", + #"pants_explorer.server", + #"internal_plugins.releases", + #"internal_plugins.test_lockfile_fixtures", ] plugins = [ "hdrhistogram", # For use with `--stats-log`. @@ -76,6 +75,12 @@ pants_ignore.add = [ build_ignore.add = [ # Disable Go targets by default so Pants developers do not need Go installed. "testprojects/src/go/**", + "testprojects/src/js/**", + "testprojects/src/jvm/**", + # Why is the docker backend under python? + "testprojects/src/python/docker/**", + "testprojects/src/ts/**", + "3rdparty/jvm/**", ] unmatched_build_file_globs = "error" @@ -203,8 +208,8 @@ requirements = ["//3rdparty/python:mypy"] [coverage-py] interpreter_constraints = ["==3.9.*"] -[preamble] -template_by_globs = "@build-support/preambles/config.yaml" +#[preamble] +#template_by_globs = "@build-support/preambles/config.yaml" [generate-lockfiles] diff = true @@ -213,30 +218,30 @@ diff = true args = ["--keep-runtime-typing", "--py38-plus"] -[jvm] -default_resolve = "jvm_testprojects" +#[jvm] +#default_resolve = "jvm_testprojects" -[jvm.resolves] +#[jvm.resolves] # A shared resolve for all testproject/example code. Because this is not shipped with Pants # binaries, it requires no isolation. -jvm_testprojects = "3rdparty/jvm/testprojects.lockfile" +#jvm_testprojects = "3rdparty/jvm/testprojects.lockfile" # A resolve for the java_parser, which is shipped with Pants, and invoked with its own isolated # classpath. Consequently, we isolate it to its own lockfile. # Note: The jvm_artifact targets in this resolve must be kept in sync with the requirements # in `generate_java_parser_lockfile_request`. -java_parser_dev = "src/python/pants/backend/java/dependency_inference/java_parser.lock" +#java_parser_dev = "src/python/pants/backend/java/dependency_inference/java_parser.lock" # Has the same isolation requirements as `java_parser`. # Note: The jvm_artifact targets in this resolve must be kept in sync with with the requirements # in `generate_scala_parser_lockfile_request`. -scala_parser_dev = "src/python/pants/backend/scala/dependency_inference/scala_parser.lock" -strip_jar_dev = "src/python/pants/jvm/strip_jar/strip_jar.lock" -jar_tool_dev = "src/python/pants/jvm/jar_tool/jar_tool.lock" +#scala_parser_dev = "src/python/pants/backend/scala/dependency_inference/scala_parser.lock" +#strip_jar_dev = "src/python/pants/jvm/strip_jar/strip_jar.lock" +#jar_tool_dev = "src/python/pants/jvm/jar_tool/jar_tool.lock" -[scala] -version_for_resolve = { "scala_parser_dev" = "2.13.8" } +#[scala] +#version_for_resolve = { "scala_parser_dev" = "2.13.8" } -[scalac] -args = ["-Yrangepos", "-Xlint:unused"] +#[scalac] +#args = ["-Yrangepos", "-Xlint:unused"] -[scala-infer] -force_add_siblings_as_dependencies = false +#[scala-infer] +#force_add_siblings_as_dependencies = false diff --git a/src/python/pants/backend/java/dependency_inference/BUILD b/src/python/pants/backend/java/dependency_inference/BUILD index d1d51d5dd1..f4d671b9e6 100644 --- a/src/python/pants/backend/java/dependency_inference/BUILD +++ b/src/python/pants/backend/java/dependency_inference/BUILD @@ -5,8 +5,8 @@ python_sources(dependencies=[":java_resources"]) resources(name="java_resources", sources=["*.java", "java_parser.lock"]) python_tests(name="tests", timeout=240) -# Targets for developing on the Java parser outside of engine rules. -java_sources( - name="java_parser", - resolve="java_parser_dev", -) +# # Targets for developing on the Java parser outside of engine rules. +# java_sources( +# name="java_parser", +# resolve="java_parser_dev", +# ) diff --git a/src/python/pants/backend/scala/dependency_inference/BUILD b/src/python/pants/backend/scala/dependency_inference/BUILD index 26b58ac2d7..2ed0e200c1 100644 --- a/src/python/pants/backend/scala/dependency_inference/BUILD +++ b/src/python/pants/backend/scala/dependency_inference/BUILD @@ -7,18 +7,18 @@ resources(name="scala_resources", sources=["*.scala", "scala_parser.lock"]) python_tests(name="tests", timeout=240) # Targets for developing on the Scala parser outside of engine rules. -scala_sources( - name="scala_parser", - resolve="scala_parser_dev", - # TODO: Allow the parser files to be formatted and linted. - skip_scalafmt=True, - skip_scalafix=True, -) +# scala_sources( +# name="scala_parser", +# resolve="scala_parser_dev", +# # TODO: Allow the parser files to be formatted and linted. +# skip_scalafmt=True, +# skip_scalafix=True, +# ) -jvm_artifact( - name="org.scala-lang_scala-library_2.13.8", - group="org.scala-lang", - artifact="scala-library", - version="2.13.8", - resolve="scala_parser_dev", -) +# jvm_artifact( +# name="org.scala-lang_scala-library_2.13.8", +# group="org.scala-lang", +# artifact="scala-library", +# version="2.13.8", +# resolve="scala_parser_dev", +# ) diff --git a/src/python/pants/core/register.py b/src/python/pants/core/register.py index 69f322e0d7..c0b721756b 100644 --- a/src/python/pants/core/register.py +++ b/src/python/pants/core/register.py @@ -6,7 +6,7 @@ These are always activated and cannot be disabled. """ from pants.backend.codegen import export_codegen_goal -from pants.bsp.rules import rules as bsp_rules +# from pants.bsp.rules import rules as bsp_rules from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.goals import ( check, @@ -86,7 +86,7 @@ def rules(): *run.rules(), *tailor.rules(), *test.rules(), - *bsp_rules(), + # *bsp_rules(), # util_rules *adhoc_binaries.rules(), *anonymous_telemetry.rules(), diff --git a/src/python/pants/goal/builtins.py b/src/python/pants/goal/builtins.py index 9b5140d0ca..e753c62c2e 100644 --- a/src/python/pants/goal/builtins.py +++ b/src/python/pants/goal/builtins.py @@ -3,7 +3,7 @@ from __future__ import annotations -from pants.bsp.goal import BSPGoal +# from pants.bsp.goal import BSPGoal from pants.build_graph.build_configuration import BuildConfiguration from pants.goal import help from pants.goal.builtin_goal import BuiltinGoal @@ -18,7 +18,7 @@ def register_builtin_goals(build_configuration: BuildConfiguration.Builder) -> N def builtin_goals() -> tuple[type[BuiltinGoal], ...]: return ( - BSPGoal, + # BSPGoal, CompletionBuiltinGoal, ExplorerBuiltinGoal, MigrateCallByNameBuiltinGoal, diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index 448b1ff540..55490ff7af 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -12,7 +12,7 @@ from pants.base.build_environment import get_buildroot from pants.base.build_root import BuildRoot from pants.base.exiter import PANTS_SUCCEEDED_EXIT_CODE from pants.base.specs import Specs -from pants.bsp.protocol import BSPHandlerMapping +# from pants.bsp.protocol import BSPHandlerMapping from pants.build_graph.build_configuration import BuildConfiguration from pants.core.util_rules import environments, system_binaries from pants.core.util_rules.environments import determine_bootstrap_environment @@ -327,10 +327,10 @@ class EngineInitializer: # Install queries for each request/response pair used by the BSP support. # Note: These are necessary because the BSP support is a built-in goal that makes # synchronous requests into the engine. - *( - QueryRule(impl.response_type, (impl.request_type, Workspace, EnvironmentName)) - for impl in union_membership.get(BSPHandlerMapping) - ), + # *( + # QueryRule(impl.response_type, (impl.request_type, Workspace, EnvironmentName)) + # for impl in union_membership.get(BSPHandlerMapping) + # ), QueryRule(Snapshot, [PathGlobs]), # Used by the SchedulerService. ) ) diff --git a/src/python/pants/jvm/jar_tool/args4j/BUILD b/src/python/pants/jvm/jar_tool/args4j/BUILD index d09026e427..9ce73736dc 100644 --- a/src/python/pants/jvm/jar_tool/args4j/BUILD +++ b/src/python/pants/jvm/jar_tool/args4j/BUILD @@ -1,6 +1,6 @@ # Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -java_sources(resolve="jar_tool_dev") +# java_sources(resolve="jar_tool_dev") resources(name="src", sources=["*.java"]) diff --git a/src/python/pants/jvm/jar_tool/jar_tool_source/BUILD b/src/python/pants/jvm/jar_tool/jar_tool_source/BUILD index d09026e427..9ce73736dc 100644 --- a/src/python/pants/jvm/jar_tool/jar_tool_source/BUILD +++ b/src/python/pants/jvm/jar_tool/jar_tool_source/BUILD @@ -1,6 +1,6 @@ # Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -java_sources(resolve="jar_tool_dev") +# java_sources(resolve="jar_tool_dev") resources(name="src", sources=["*.java"]) diff --git a/src/python/pants/jvm/strip_jar/BUILD b/src/python/pants/jvm/strip_jar/BUILD index 85fff62636..38dbfd8bc2 100644 --- a/src/python/pants/jvm/strip_jar/BUILD +++ b/src/python/pants/jvm/strip_jar/BUILD @@ -6,16 +6,16 @@ resources(name="java_resources", sources=["*.java", "strip_jar.lock"]) python_tests(name="tests", timeout=240) # Targets for developing on the strip jar outside of engine rules. -java_sources( - name="strip_jar_java", - resolve="strip_jar_dev", -) +# java_sources( +# name="strip_jar_java", +# resolve="strip_jar_dev", +# ) -jvm_artifact( - name="reproducible-builds-jvm-stripper", - group="io.github.zlika", - artifact="reproducible-build-maven-plugin", - version="0.16", - resolve="strip_jar_dev", - packages=["io.github.zlika.reproducible.**"], -) +# jvm_artifact( +# name="reproducible-builds-jvm-stripper", +# group="io.github.zlika", +# artifact="reproducible-build-maven-plugin", +# version="0.16", +# resolve="strip_jar_dev", +# packages=["io.github.zlika.reproducible.**"], +# ) ```

sureshjoshi commented 3 months ago

I've made suggestions in the slack chat, but essentially, I'm curious if we can kick any of that to pants.ci.toml, opt-in disable it, or just remove it entirely.

cognifloyd commented 3 months ago

16445 is a more general issue about allowing people to not load language backends they don't need. That issue is for general pants usage, while this issue is about developing pants itself. The solution for #16445 would probably be a big part of fixing this.

sureshjoshi commented 3 months ago

The solution for https://github.com/pantsbuild/pants/issues/16445 would probably be a big part of fixing this.

I agree, or in general, some way to fully lazy-load backends - so unless your selected target requires the backend - it's basically ignored. But, chicken and egg, without loading the backend, how can you identify target types

huonw commented 3 months ago

We've talked previously about using Pants to build Pants ("bootstrapping"):

This might help solve this problem, in some ways, by dramatically reducing how often the scheduler is invalidated and restarted. Editing the Python backend for the Pants-in-repository doesn't require restarting the orchestrating-Pants scheduler, because orchestrating-Pants is using its own backend. (e.g. if this repo is configured to use 2.21.0, it'll just be using the 2.21 version of the Pants backends)

The work on the new workspace_environment might make this easy: we can run cargo to build the native parts in the workspace, solving some of the performance issues that were previously blocking it.

sureshjoshi commented 3 months ago

I'd be a huge fan of treating pants like a pants repo. And I think Stu's suggestion of having a "binary engine mode" against a "source engine mode" would make sense as the degenerate case. I mean, in-repo plugins can kinda do that already using PANTS_SOURCE - so this would be pants sourcing itself 🤷🏽

pantsbuild / pants

Mainline pants has a slow development cycle #21096

16445 is a more general issue about allowing people to not load language backends they don't need. That issue is for general pants usage, while this issue is about developing pants itself. The solution for #16445 would probably be a big part of fixing this.