Skip to content

Commit

Permalink
Consolidate WindowsWmicFinder (#25)
Browse files Browse the repository at this point in the history
- Move the `popen` function check instead of relying on the `CpuCoreCounter`'s `proc_open` check.
- Return `null` directly if is not on Windows (hence the command will not work); this was previously done in `CpuCoreCounter` but it more belongs to the finder as it's really a micro-optimization since the constant defined check is faster than doing `popen`
- Refactor `WindowsWmicFinder::find()` for better readability
- Handle potential `fgets` `false` value returned
  • Loading branch information
theofidry authored Dec 4, 2022
1 parent 6b8549b commit f1ea48c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 18 deletions.
13 changes: 3 additions & 10 deletions src/CpuCoreCounter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
namespace Fidry\CpuCounter;

use function function_exists;
use const DIRECTORY_SEPARATOR;

final class CpuCoreCounter
{
Expand Down Expand Up @@ -76,16 +75,10 @@ private function findCount(): int
public static function getDefaultFinders(): array
{
/** @var list<class-string<CpuCoreFinder>> $finders */
$finders = [
return [
new CpuInfoFinder(),
new WindowsWmicFinder(),
new HwFinder(),
];

if (DIRECTORY_SEPARATOR === '\\') {
$finders[] = new WindowsWmicFinder();
}

$finders[] = new HwFinder();

return $finders;
}
}
25 changes: 17 additions & 8 deletions src/WindowsWmicFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@

namespace Fidry\CpuCounter;

use function defined;
use function fgets;
use function filter_var;
use function function_exists;
use function is_int;
use function is_resource;
use function pclose;
Expand All @@ -34,18 +36,25 @@ final class WindowsWmicFinder implements CpuCoreFinder
*/
public function find(): ?int
{
// Windows
$process = popen('wmic cpu get NumberOfLogicalProcessors', 'rb');
if (!function_exists('popen')
|| !defined('PHP_WINDOWS_VERSION_MAJOR')
) {
return null;
}

if (is_resource($process)) {
fgets($process);
$cores = self::countCpuCores(fgets($process));
pclose($process);
// -n to show only the variable value
$process = popen('wmic cpu get NumberOfLogicalProcessors', 'rb');

return $cores;
if (!is_resource($process)) {
return null;
}

return null;
$processResult = fgets($process);
pclose($process);

return false === $processResult
? null
: self::countCpuCores($processResult);
}

/**
Expand Down

0 comments on commit f1ea48c

Please sign in to comment.