Open divinity76 opened 6 months ago
My PATH
has 21 folders, that would result in 84 checks (21 x 4) in the worst case. I know that command -v
probably does the same thing under the hood, but it feels so wrong to scan folders manually like this π
My
PATH
has 21 folders, that would result in 84 checks (21 x 4) in the worst case. I know thatcommand -v
probably does the same thing under the hood, but it feels so wrong to scan folders manually like this π
yeah i know what you meanπ i have 25 here (WSL Ubuntu 24.04-beta)
Benchmarking it on a AMD Ryzen 9 7950x (high-end 2022 system) Ubuntu 24.04-beta WSL:
$ sudo bash -c 'sync; echo 3 > /proc/sys/vm/drop_caches';
$ time bash -c ' command -v google-chrome chromium-browser chrome chromium'
/usr/bin/chromium-browser
/snap/bin/chromium
real 0m0.024s
user 0m0.003s
sys 0m0.000s
$ time bash -c ' command -v google-chrome chromium-browser chrome chromium'
/usr/bin/chromium-browser
/snap/bin/chromium
real 0m0.022s
user 0m0.003s
sys 0m0.000s
$ sudo bash -c 'sync; echo 3 > /proc/sys/vm/drop_caches';
$ time php -r '
$a=explode(PATH_SEPARATOR, getenv("PATH"));
var_dump(count($a));
$names=["google-chrome","chromium-browser","chrome","chromium"];
foreach($a as $dir){
foreach($names as $name){
$file=$dir.DIRECTORY_SEPARATOR.$name;
if(file_exists($file) && is_executable($file)){
echo $file."\n";
}
}
}
';
Command line code:3:
int(25)
/usr/bin/chromium-browser
/bin/chromium-browser
/snap/bin/chromium
real 0m0.097s
user 0m0.015s
sys 0m0.007s
$ time php -r '
$a=explode(PATH_SEPARATOR, getenv("PATH"));
var_dump(count($a));
$names=["google-chrome","chromium-browser","chrome","chromium"];
foreach($a as $dir){
foreach($names as $name){
$file=$dir.DIRECTORY_SEPARATOR.$name;
if(file_exists($file) && is_executable($file)){
echo $file."\n";
}
}
}
';
Command line code:3:
int(25)
/usr/bin/chromium-browser
/bin/chromium-browser
/snap/bin/chromium
real 0m0.041s
user 0m0.015s
sys 0m0.000s
(php does more work than command here because php does not bail, it simulates absolutely worst case scenario, command does not)
doing the same on a Intel i7-8565U, a 2018 15watt low-power laptop cpu running on battery in power-saving mode, on WSL Ubuntu 24.04-beta:
hans@DESKTOP-2LHJILI:~$ sudo bash -c 'sync; echo 3 > /proc/sys/vm/drop_caches';
hans@DESKTOP-2LHJILI:~$ time bash -c ' command -v google-chrome chromium-browser chrome chromium'
/usr/bin/chromium-browser
/snap/bin/chromium
real 0m0.820s
user 0m0.001s
sys 0m0.050s
hans@DESKTOP-2LHJILI:~$ time bash -c ' command -v google-chrome chromium-browser chrome chromium'
/usr/bin/chromium-browser
/snap/bin/chromium
real 0m0.476s
user 0m0.001s
sys 0m0.027s
hans@DESKTOP-2LHJILI:~$ sudo bash -c 'sync; echo 3 > /proc/sys/vm/drop_caches';
hans@DESKTOP-2LHJILI:~$ time php -r '
$a=explode(PATH_SEPARATOR, getenv("PATH"));
var_dump(count($a));
$names=["google-chrome","chromium-browser","chrome","chromium"];
foreach($a as $dir){
foreach($names as $name){
$file=$dir.DIRECTORY_SEPARATOR.$name;
if(file_exists($file) && is_executable($file)){
echo $file."\n";
}
}
}
';
int(32)
/usr/bin/chromium-browser
/bin/chromium-browser
/snap/bin/chromium
real 0m1.168s
user 0m0.062s
sys 0m0.076s
hans@DESKTOP-2LHJILI:~$ time php -r '
$a=explode(PATH_SEPARATOR, getenv("PATH"));
var_dump(count($a));
$names=["google-chrome","chromium-browser","chrome","chromium"];
foreach($a as $dir){
foreach($names as $name){
$file=$dir.DIRECTORY_SEPARATOR.$name;
if(file_exists($file) && is_executable($file)){
echo $file."\n";
}
}
}
';
int(32)
/usr/bin/chromium-browser
/bin/chromium-browser
/snap/bin/chromium
real 0m0.617s
user 0m0.030s
sys 0m0.030s
π€ i think servers will do just fine. laptop users should specify chrome path to avoid the delay
could be cached, but then we'd need to consider cache invalidation and cache poisoning π€
The performance will always be ok since command
do the same thing, it just feels wrong π
. The code itself seems to work. If I have time in the next few days I'll see if there is any other better way of doing this.
Php already has the stat cache. It won't do anything in this case but it's still not an issue.
I'm not really sure we should be making auto-discover that complicated. The point is to detect the most common places, only. I feel like people should be passing in the binary path explicitly, otherwise, or implementing this kind of logic in their own app.
@GrahamCampbell so you're basically proposing
$commonLocations = array(
"/usr/bin/chromium-browser",
"/bin/chromium-browser",
"/snap/bin/chromium",
(...)
);
ForEach($commonLocations as (...)
?
I couldn't think of any better way to implement this and I recreating the command
behaviour seems out of the scope of this project, but if you manage to convince Graham I'm ok with it.
command is not always available in shell_exec() , observed on Ubuntu-24.04-beta + PHP8.3 see https://github.com/chrome-php/chrome/issues/613 for details