Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mainline pants has a slow development cycle #21096

Open
sureshjoshi opened this issue Jun 21, 2024 · 5 comments
Open

Mainline pants has a slow development cycle #21096

sureshjoshi opened this issue Jun 21, 2024 · 5 comments
Labels

Comments

@sureshjoshi
Copy link
Member

sureshjoshi commented Jun 21, 2024

Pulled from the associated slack chat.

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
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
Copy link
Member Author

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
Copy link
Member

cognifloyd commented Jun 21, 2024

#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
Copy link
Member Author

The solution for #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
Copy link
Contributor

huonw commented Jun 21, 2024

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
Copy link
Member Author

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 🤷🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants