buck2: hybrid: don't cap permits at machine size for the low pass filter

Summary:
Like it says in the title. We don't actually care to exceed the machine
capacity in the low pass filter (in that case we *would* like to not run
anything locally: there is nothing special about *1* task).

This notably makes it easier to add tests (see next diff).

Reviewed By: get9

Differential Revision: D47406600

fbshipit-source-id: c16003a7277e097dbbab0fb1529a8395595cf603

Author: krallin

app/buck2_execute_impl/src/executors/hybrid.rs

@@ -204,12 +204,16 @@ where
 
         let weight = match command.request.host_sharing_requirements() {
             HostSharingRequirements::ExclusiveAccess => self.low_pass_filter.capacity(),
-            HostSharingRequirements::OnePerToken(.., class) => {
-                self.local.host_sharing_broker.requested_permits(class)
-            }
-            HostSharingRequirements::Shared(class) => {
-                self.local.host_sharing_broker.requested_permits(class)
-            }
+            HostSharingRequirements::OnePerToken(.., class) => self
+                .local
+                .host_sharing_broker
+                .requested_permits(class)
+                .into_count_uncapped(),
+            HostSharingRequirements::Shared(class) => self
+                .local
+                .host_sharing_broker
+                .requested_permits(class)
+                .into_count_uncapped(),
         };
 
         let is_retryable_status = move |r: &CommandExecutionResult| {

host_sharing/src/host_sharing.rs

@@ -110,18 +110,36 @@ pub struct HostSharingBroker {
     named_semaphores: NamedSemaphores,
 }
 
+pub struct RequestedPermits {
+    count: usize,
+    cap: usize,
+}
+
+impl RequestedPermits {
+    pub fn into_count(self) -> usize {
+        self.count.min(self.cap)
+    }
+
+    pub fn into_count_uncapped(self) -> usize {
+        self.count
+    }
+}
+
 impl HostSharingBroker {
     // If a test requires Permits(4) permits but the machine only has 3 permits then we cap the
     // test's required permits to 3. Otherwise the test would never be allowed to run.
-    pub fn requested_permits(&self, weight_class: &WeightClass) -> usize {
-        match weight_class {
-            WeightClass::Permits(required_permits) => {
-                self.num_machine_permits.min(*required_permits)
-            }
+    pub fn requested_permits(&self, weight_class: &WeightClass) -> RequestedPermits {
+        let count = match weight_class {
+            WeightClass::Permits(required_permits) => *required_permits,
             WeightClass::Percentage(percentage) => {
                 let percentage: usize = percentage.into_value().into();
                 (self.num_machine_permits * percentage).div_ceil(100)
             }
+        };
+
+        RequestedPermits {
+            count,
+            cap: self.num_machine_permits,
         }
     }
 
@@ -150,7 +168,7 @@ impl HostSharingBroker {
     ) -> HostSharingGuard {
         match host_sharing_requirements {
             HostSharingRequirements::Shared(weight_class) => {
-                let permits = self.requested_permits(weight_class);
+                let permits = self.requested_permits(weight_class).into_count();
                 let _run_guard = self.permits.acquire(permits).await;
                 HostSharingGuard {
                     _run_guard,
@@ -171,7 +189,7 @@ impl HostSharingBroker {
                 // for the previous run on this identifier to finish.
                 let run_semaphore = self.named_semaphores.get(identifier);
                 let _name_guard = Some(run_semaphore.acquire(SINGLE_RUN).await);
-                let permits = self.requested_permits(weight_class);
+                let permits = self.requested_permits(weight_class).into_count();
                 let _run_guard = self.permits.acquire(permits).await;
                 HostSharingGuard {
                     _run_guard,
@@ -198,37 +216,54 @@ mod tests {
     fn test_heavyweight_capped_to_machine_permits() {
         let broker = HostSharingBroker::new(HostSharingStrategy::SmallerTasksFirst, 2);
 
-        let permits = broker.requested_permits(&WeightClass::Permits(4));
+        let permits = broker
+            .requested_permits(&WeightClass::Permits(4))
+            .into_count();
         assert_eq!(2, permits);
+
+        let permits = broker
+            .requested_permits(&WeightClass::Permits(4))
+            .into_count_uncapped();
+        assert_eq!(4, permits);
     }
 
     #[test]
     fn test_percentage() {
         let broker = HostSharingBroker::new(HostSharingStrategy::SmallerTasksFirst, 10);
 
         assert_eq!(
-            broker.requested_permits(&WeightClass::Percentage(WeightPercentage { value: 0 })),
+            broker
+                .requested_permits(&WeightClass::Percentage(WeightPercentage { value: 0 }))
+                .into_count(),
             0,
         );
 
         assert_eq!(
-            broker.requested_permits(&WeightClass::Percentage(WeightPercentage { value: 40 })),
+            broker
+                .requested_permits(&WeightClass::Percentage(WeightPercentage { value: 40 }))
+                .into_count(),
             4,
         );
 
         // This rounds up.
         assert_eq!(
-            broker.requested_permits(&WeightClass::Percentage(WeightPercentage { value: 15 })),
+            broker
+                .requested_permits(&WeightClass::Percentage(WeightPercentage { value: 15 }))
+                .into_count(),
             2,
         );
 
         assert_eq!(
-            broker.requested_permits(&WeightClass::Percentage(WeightPercentage { value: 99 })),
+            broker
+                .requested_permits(&WeightClass::Percentage(WeightPercentage { value: 99 }))
+                .into_count(),
             10,
         );
 
         assert_eq!(
-            broker.requested_permits(&WeightClass::Percentage(WeightPercentage { value: 100 })),
+            broker
+                .requested_permits(&WeightClass::Percentage(WeightPercentage { value: 100 }))
+                .into_count(),
             10,
         );
     }