Skylark debugging protocol: only track paused or stepping threads.

- remove blaze-side thread creation/destruction hooks
- remove ListThreads, ThreadStarted, ThreadEnded events from protocol
- don't track unpaused, not stepping threads in ThreadHandler

The threading model didn't provide useful information -- in practice,
users never want to list currently-running threads, and the debug
client doesn't need to list currently-paused threads, since it receives
each ThreadPaused and ThreadContinued event, so its model is always up to date.

Not tracking thread creation/destruction greatly simplifies the API, and
reduces the blaze-side hooks -- it was also only semi-implemented, as
plenty (/most?) skylark threads were never registered.

The biggest cost to removing this is lack of nice thread names. In
practice, I think the benefits greatly outweigh this cost. If it ever
becomes a problem, there are other lighter-weight ways of communicating
thread names.

TAG_CHANGE_OK=This proto has never yet been used
TYPE_CHANGE_OK=This proto has never yet been used
PiperOrigin-RevId: 201532462

1 files changed, 9 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index fa0eefb361..1bb02261b2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -29,7 +29,6 @@ import com.google.devtools.build.lib.packages.Info;
 import com.google.devtools.build.lib.packages.SkylarkDefinedAspect;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
-import com.google.devtools.build.lib.syntax.DebugServerUtils;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace;
@@ -71,25 +70,16 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
               // NB: loading phase functions are not available: this is analysis already, so we do
               // *not* setLoadingPhase().
               .build();
+      Object aspectSkylarkObject;
       try {
-        final SkylarkRuleContext finalRuleContext = skylarkRuleContext;
-        Object aspectSkylarkObject =
-            DebugServerUtils.runWithDebuggingIfEnabled(
-                env,
-                () ->
-                    String.format(
-                        "Aspect %s on %s",
-                        skylarkAspect.getName(),
-                        ruleContext.getTarget().getLabel().getCanonicalForm()),
-                () ->
-                    skylarkAspect
-                        .getImplementation()
-                        .call(
-                            /*args=*/ ImmutableList.of(
-                                ctadBase.getConfiguredTarget(), finalRuleContext),
-                            /* kwargs= */ ImmutableMap.of(),
-                            /*ast=*/ null,
-                            env));
+        aspectSkylarkObject =
+            skylarkAspect
+                .getImplementation()
+                .call(
+                    /*args=*/ ImmutableList.of(ctadBase.getConfiguredTarget(), skylarkRuleContext),
+                    /* kwargs= */ ImmutableMap.of(),
+                    /*ast=*/ null,
+                    env);
 
         if (ruleContext.hasErrors()) {
           return null;