From c560bac13f9b44885a3fec3b82a1ef674dc30f1d Mon Sep 17 00:00:00 2001
From: Ian Manske <ian.manske@pm.me>
Date: Sun, 1 Dec 2024 03:56:42 -0800
Subject: [PATCH] Add `--long` flag for `sys cpu` (#14485)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Description

Fixes #14470 where the `sys cpu` command is slow. This was done by
removing the `cpu_usage` column from the default output, since it takes
400ms to calculate. Instead a `--long` flag was added that, when
provided, adds back the `cpu_usage` column.

```nu
# Before
> bench { sys cpu | length } | get mean
401ms 591µs 896ns

# After
> bench { sys cpu | length } | get mean
500µs 13ns # around 1-2ms in practice
```

# User-Facing Changes

- `sys cpu` no longer has a `cpu_usage` column by default.
- Added  a `--long` flag for `sys cpu` to add back the removed column.
---
 crates/nu-command/src/system/sys/cpu.rs | 48 +++++++++++++++----------
 crates/nu-engine/src/command_prelude.rs |  4 +--
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/crates/nu-command/src/system/sys/cpu.rs b/crates/nu-command/src/system/sys/cpu.rs
index 3243478e03..de24efd898 100644
--- a/crates/nu-command/src/system/sys/cpu.rs
+++ b/crates/nu-command/src/system/sys/cpu.rs
@@ -13,6 +13,11 @@ impl Command for SysCpu {
     fn signature(&self) -> Signature {
         Signature::build("sys cpu")
             .filter()
+            .switch(
+                "long",
+                "Get all available columns (slower, needs to sample CPU over time)",
+                Some('l'),
+            )
             .category(Category::System)
             .input_output_types(vec![(Type::Nothing, Type::table())])
     }
@@ -23,12 +28,13 @@ impl Command for SysCpu {
 
     fn run(
         &self,
-        _engine_state: &EngineState,
-        _stack: &mut Stack,
+        engine_state: &EngineState,
+        stack: &mut Stack,
         call: &Call,
         _input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        Ok(cpu(call.head).into_pipeline_data())
+        let long = call.has_flag(engine_state, stack, "long")?;
+        Ok(cpu(long, call.head).into_pipeline_data())
     }
 
     fn examples(&self) -> Vec<Example> {
@@ -40,38 +46,44 @@ impl Command for SysCpu {
     }
 }
 
-fn cpu(span: Span) -> Value {
+fn cpu(long: bool, span: Span) -> Value {
     let mut sys = System::new();
-    sys.refresh_cpu_specifics(CpuRefreshKind::everything());
-    // We must refresh the CPU twice a while apart to get valid usage data.
-    // In theory we could just sleep MINIMUM_CPU_UPDATE_INTERVAL, but I've noticed that
-    // that gives poor results (error of ~5%). Decided to wait 2x that long, somewhat arbitrarily
-    std::thread::sleep(MINIMUM_CPU_UPDATE_INTERVAL * 2);
-    sys.refresh_cpu_specifics(CpuRefreshKind::new().with_cpu_usage());
+    if long {
+        sys.refresh_cpu_specifics(CpuRefreshKind::everything());
+        // We must refresh the CPU twice a while apart to get valid usage data.
+        // In theory we could just sleep MINIMUM_CPU_UPDATE_INTERVAL, but I've noticed that
+        // that gives poor results (error of ~5%). Decided to wait 2x that long, somewhat arbitrarily
+        std::thread::sleep(MINIMUM_CPU_UPDATE_INTERVAL * 2);
+        sys.refresh_cpu_specifics(CpuRefreshKind::new().with_cpu_usage());
+    } else {
+        sys.refresh_cpu_specifics(CpuRefreshKind::new().with_frequency());
+    }
 
     let cpus = sys
         .cpus()
         .iter()
         .map(|cpu| {
-            // sysinfo CPU usage numbers are not very precise unless you wait a long time between refreshes.
-            // Round to 1DP (chosen somewhat arbitrarily) so people aren't misled by high-precision floats.
-            let rounded_usage = (cpu.cpu_usage() * 10.0).round() / 10.0;
-
             let load_avg = System::load_average();
             let load_avg = format!(
                 "{:.2}, {:.2}, {:.2}",
                 load_avg.one, load_avg.five, load_avg.fifteen
             );
 
-            let record = record! {
+            let mut record = record! {
                 "name" => Value::string(trim_cstyle_null(cpu.name()), span),
                 "brand" => Value::string(trim_cstyle_null(cpu.brand()), span),
-                "freq" => Value::int(cpu.frequency() as i64, span),
-                "cpu_usage" => Value::float(rounded_usage.into(), span),
-                "load_average" => Value::string(load_avg, span),
                 "vendor_id" => Value::string(trim_cstyle_null(cpu.vendor_id()), span),
+                "freq" => Value::int(cpu.frequency() as i64, span),
+                "load_average" => Value::string(load_avg, span),
             };
 
+            if long {
+                // sysinfo CPU usage numbers are not very precise unless you wait a long time between refreshes.
+                // Round to 1DP (chosen somewhat arbitrarily) so people aren't misled by high-precision floats.
+                let rounded_usage = (f64::from(cpu.cpu_usage()) * 10.0).round() / 10.0;
+                record.push("cpu_usage", rounded_usage.into_value(span));
+            }
+
             Value::record(record, span)
         })
         .collect();
diff --git a/crates/nu-engine/src/command_prelude.rs b/crates/nu-engine/src/command_prelude.rs
index e6ddb5fb91..0c5c250a27 100644
--- a/crates/nu-engine/src/command_prelude.rs
+++ b/crates/nu-engine/src/command_prelude.rs
@@ -3,6 +3,6 @@ pub use nu_protocol::{
     ast::CellPath,
     engine::{Call, Command, EngineState, Stack, StateWorkingSet},
     record, ByteStream, ByteStreamType, Category, ErrSpan, Example, IntoInterruptiblePipelineData,
-    IntoPipelineData, IntoSpanned, PipelineData, Record, ShellError, Signature, Span, Spanned,
-    SyntaxShape, Type, Value,
+    IntoPipelineData, IntoSpanned, IntoValue, PipelineData, Record, ShellError, Signature, Span,
+    Spanned, SyntaxShape, Type, Value,
 };