Remove old default critical path

Summary: Let's remove the old default backend for critical path. Currently, we have longest-path-graph backend (which stores potentials) almost everywhere in the codebase. We should use that one everywhere and there is no need for the default anymore since it has less functionality.

Reviewed By: scottcao

Differential Revision: D68267529

fbshipit-source-id: 61b09255f83b6d07a144b59b685216173aa3acfb

Author: ezgicicek

app/buck2_build_signals/src/env.rs

@@ -50,10 +50,9 @@ impl NodeDuration {
 
 #[derive(Copy, Clone, Dupe, derive_more::Display, Allocative)]
 pub enum CriticalPathBackendName {
+    /// This is the default backend.
     #[display("longest-path-graph")]
     LongestPathGraph,
-    #[display("default")]
-    Default,
     #[display("logging")]
     Logging,
 }
@@ -64,13 +63,7 @@ impl FromStr for CriticalPathBackendName {
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         if s == "longest-path-graph" {
             return Ok(Self::LongestPathGraph);
-        }
-
-        if s == "default" {
-            return Ok(Self::Default);
-        }
-
-        if s == "logging" {
+        } else if s == "logging" {
             return Ok(Self::Logging);
         }
 

app/buck2_build_signals_impl/src/backend.rs

@@ -8,6 +8,5 @@
  */
 
 pub(crate) mod backend;
-pub(crate) mod default;
 pub(crate) mod logging;
 pub(crate) mod longest_path_graph;

app/buck2_build_signals_impl/src/backend/default.rs

@@ -64,7 +64,6 @@ use tokio_stream::wrappers::UnboundedReceiverStream;
 use tokio_stream::StreamExt;
 
 use crate::backend::backend::BuildListenerBackend;
-use crate::backend::default::DefaultBackend;
 use crate::backend::logging::LoggingBackend;
 use crate::backend::longest_path_graph::LongestPathGraphBackend;
 
@@ -322,9 +321,6 @@ impl DeferredBuildSignals for DeferredBuildSignalsImpl {
             CriticalPathBackendName::LongestPathGraph => {
                 start_backend(events, self.receiver, LongestPathGraphBackend::new(), ctx)
             }
-            CriticalPathBackendName::Default => {
-                start_backend(events, self.receiver, DefaultBackend::new(), ctx)
-            }
             CriticalPathBackendName::Logging => start_backend(
                 events.dupe(),
                 self.receiver,

app/buck2_build_signals_impl/src/lib.rs

@@ -738,7 +738,7 @@ impl<'a, 's> DiceCommandUpdater<'a, 's> {
                 section: "buck2",
                 property: "critical_path_backend2",
             })?
-            .unwrap_or(CriticalPathBackendName::Default);
+            .unwrap_or(CriticalPathBackendName::LongestPathGraph);
 
         let override_use_case = root_config.parse::<RemoteExecutorUseCase>(BuckconfigKeyRef {
             section: "buck2_re_client",

app/buck2_server/src/ctx.rs

@@ -31,7 +31,7 @@ class critical_path_log:
     potential_improvement_duration: str
 
 
-async def do_critical_path(buck: Buck, correct_analysis: bool) -> None:
+async def do_critical_path(buck: Buck) -> None:
     await buck.build("//:step_3", "--no-remote-cache")
 
     critical_path = (await buck.log("critical-path")).stdout.strip().splitlines()
@@ -57,13 +57,7 @@ async def do_critical_path(buck: Buck, correct_analysis: bool) -> None:
         ("materialization", "root//:step_3"),
         ("compute-critical-path", ""),
     ]
-    if correct_analysis:
-        assert len(critical_path) == len(expected)
-    else:
-        # If correct_analysis = False (i.e. backend is the default), analysis nodes are not handled properly.
-        # There is now non-determinism in this test since what we get back depends on
-        # where the analysis becomes the longest path.
-        assert len(trimmed_critical_path) > 0
+    assert len(critical_path) == len(expected)
 
     for s, e in zip(reversed(trimmed_critical_path), reversed(expected)):
         if s.kind == "action":
@@ -72,23 +66,16 @@ async def do_critical_path(buck: Buck, correct_analysis: bool) -> None:
         else:
             assert s.execution_kind == ""
 
-        if not correct_analysis and s.kind == "analysis":
-            break
         assert s.kind == e[0]
         assert s.name == e[1]
 
 
-@buck_test()
-async def test_critical_path(buck: Buck) -> None:
-    await do_critical_path(buck, False)
-
-
 @buck_test()
 async def test_critical_path_longest_path_graph(buck: Buck) -> None:
     with open(buck.cwd / ".buckconfig", "a") as f:
         f.write("[buck2]\n")
         f.write("critical_path_backend2 = longest-path-graph\n")
-    await do_critical_path(buck, True)
+    await do_critical_path(buck)
 
 
 @buck_test()
@@ -103,7 +90,6 @@ async def test_critical_path_json(buck: Buck) -> None:
     )
     critical_path = [json.loads(e) for e in critical_path]
 
-    assert len(critical_path) > 0
     expected = [
         ("time-spent-synchronizing-and-waiting", None),
         ("listing", "root//"),
@@ -119,15 +105,9 @@ async def test_critical_path_json(buck: Buck) -> None:
         ("materialization", "root//:step_3"),
         ("compute-critical-path", None),
     ]
+    assert len(critical_path) == len(expected)
 
     for critical, exp in zip(reversed(critical_path), reversed(expected)):
-        if critical["kind"] == "analysis":
-            # When the backend is the default, analysis nodes are not handled properly.
-            # There is now non-determinism in this test since what we get back depends on
-            # where the analysis becomes the longest path.
-            # This is fixed when critical_path_backend2 = longest-path-graph.
-            break
-
         assert "kind" in critical
         assert critical["kind"] == exp[0]