A short script to remove unused, useless globals in PHP.

Nao

  • Dadman with a boy
  • Posts: 16,077
A short script to remove unused, useless globals in PHP.
« on November 16th, 2013, 01:03 AM »Last edited on November 16th, 2013, 12:19 PM
As mentioned in the thoughts area, I quickly wrote a script to find unused variables declared with the global keyword in your PHP scripts.

Just copy this into any PHP file, drop it at the root of your website install, and have fun! It's not 100% tested, but it seems to be working fine, so I thought I'd share it, if only because SMF needs to do something about their 1600+ unused globals. I know they don't take much processing power or anything, but it just sounds lazy to me to leave globals that are unused.

Feel free to improve it as you like!
I'm releasing this under the WTF Public License, for what it's worth. It's not long enough to even bother with a MIT license, although I dream of using that one in the future... :P

Code: [Select]
<?php

find_unused_globals
(dirname(__FILE__));

function 
find_unused_globals($dir)
{
$i 0;
$files scandir($dir);
foreach ($files as $file)
{
if ($file == '.' || $file == '..' || $file == '.git' || $file == 'other')
continue;
if (is_dir($dir '/' $file))
{
find_unused_globals($dir '/' $file);
continue;
}
if (substr($file, -4) != '.php')
continue;
$php file_get_contents($dir '/' $file);
preg_match_all('~\n(\t*)function ([\w]+)[^}\n]+\n\1(.*?)\n\1}~s'$php$matches);
foreach ($matches[3] as $key => $val)
{
preg_match_all('~global (\$[^;]+);~'$val$globs);
$globs[1] = isset($globs[1]) ? $globs[1] : array();

foreach ($globs[1] as $find_dupes)
{
$dupes array_map('trim'explode(','$find_dupes));
if (count($dupes) > count(array_flip(array_flip($dupes))))
echo 'Found duplicate globals in '$file':'$matches[2][$key], ' -- '$find_dupes"\n";
}

preg_match_all('~\$[a-zA-Z_]+~'implode(', '$globs[1]), $there_we_are);
$val str_replace($globs[0], ''$val);
if (isset($there_we_are[0]))
foreach ($there_we_are[0] as $test_me)
if (strpos($val$test_me) === false)
echo 'Unused global in '$file':'$matches[2][$key], ' -- '$test_me"\n";
}
}
}

To do the opposite operation (i.e. finding globals that weren't declared in a global keyword) isn't going to work with a short script -- unless you only look for the 'usual culprits' ($context, $txt, etc), in which case, yes, a short script is doable, I guess. But for that, I'm using HHVM. It's working fine, and is very smart about it. Only problem, it only works on Linux, so I had to install a VM to run that VM. Amusing. And install tons of dependencies. And go through many more steps of the program not wanting to run (the joys of permissions), then not wanting to start operating (the joys of parameters), then not wanting to complete (the joys of JSON bugs in Ubuntu.)
It took me several hours to go through it, but I found over 400 documented 'bugs' in Wedge, some of which I believe are in SMF/Elk as well, not that many though, but I'll try to document those I find important for them to fix.

:edit:
- Fixed most single-line function declarations.
- Added a 'duplicate global definitions' part. If you have a line that says global $txt, $context, $txt, the script will now tell you about it.
Re: A short script to remove unused, useless globals in PHP.
« Reply #1, on November 16th, 2013, 09:08 AM »
I fixed a small bug in which unused globals were reported in wrong situations.

Current results:
Elk - 159 declared, but unused globals. Good job. I suspect this is the work of TE using NetBeans :P
Wedge - 323 declared, but unused globals. I'll get started on this! (I removed the /other/ folder from results, because it's a large offender, and will never make it into the Wedge package.)
SMF 2.1 - 1231 declared, but unused globals. Baaaaad!

Re: A short script to remove unused, useless globals in PHP.
« Reply #2, on November 16th, 2013, 11:57 AM »
Very interesting! I have been lazy with this..well, some time ago I started being more observant in my own functions, but I haven't touched any of the SMF native in this manner.

The results I got..(differs from yours? could be my changed script..?
Protendo    2088     its built on SMF 2.0.x so to be expected I guess. Still bad though.
Wedge    542     but I suspect your current working copy have even fewer.
Elkarte    83     Could be from using more OOP perhaps?
SMF2.1    1231     not bad really, but still behind Elkarte/wedge
Silvercircle    1316     not bad
SMF 2.0.6    2500     this I expected hah!


(is there some styling in the table tag in wedge btw? I added a few basic color schemes in Protendo,in  a new table-like tag with headers and title as well, very useful for me at least :D ).

Having unused globals takes memory, right? Thats the main point in removing them?

oh, and I added some to the script to read the results better(for me) so it now just reads:
Code: [Select]
<?php

$i 
0;
find_unused_globals(dirname(__FILE__));
echo 
'<hr>total: '.$i.'';


function 
find_unused_globals($dir)
{
global $i;
$files scandir($dir);
foreach ($files as $file)
{
if ($file == '.' || $file == '..' || $file == '.git')
continue;
if (is_dir($dir '/' $file))
{
find_unused_globals($dir '/' $file);
continue;
}
if (substr($file, -4) != '.php')
continue;
$php file_get_contents($dir '/' $file);
preg_match_all('~\n(\t*)function ([\w]+)[^\n]+\n\1(.*?)\n\1}~s'$php$matches);
foreach ($matches[3] as $key => $val)
{
preg_match_all('~global ([^;]+);~'$val$globs);
$glos = isset($globs[1]) ? implode(','$globs[1]) : '';
preg_match_all('~\$[a-zA-Z_]+~'$glos$there_we_are);
$val str_replace($globs[0], ''$val);
if (isset($there_we_are[0]))
foreach ($there_we_are[0] as $test_me)
if (strpos($val$test_me) === false)
{
echo '<div style="margin: 1px 0; border: solid 1px #aaa;background: #eee; padding: 5px 10px;">Unused global in '$file':'$matches[2][$key], ' -- '$test_me"</div>";
$i++;
}
}
}
}
Re: A short script to remove unused, useless globals in PHP.
« Reply #3, on November 16th, 2013, 12:02 PM »
Come to think of it - I am illiterate at this really - the use of objects, they also take up memory..so could Elkarte with so few globals in fact be using just as much memory as SMF due to them? Maybe some benchmark/resource meter for PHP will show this, I need to investigate further.

Nao

  • Dadman with a boy
  • Posts: 16,077
Re: A short script to remove unused, useless globals in PHP.
« Reply #4, on November 16th, 2013, 12:27 PM »
Quote from Bloc on November 16th, 2013, 11:57 AM
Very interesting! I have been lazy with this..
No problem being lazy; you just need to fix when you spot it. :P
And with this script, then everyone can afford to be lazy again, in the future. They can just whip it up at some point, and fix everything they forgot to fix in one go.
Quote
The results I got..(differs from yours? could be my changed script..?
Lulz Protendo results... ;)
It's no problem, though.
In fact, I don't think adding tons of unused globals will have any performance impact. It's just, after all, an 'alias' to something that can be accessed through $GLOBALS&#91;'var'] just the same.
I do think, however, that it's best to save on filesize as much as possible. Every byte always counts, in the long run. Your mileage may vary.

My working copy had fewer than 542, because I'd already started removing a few ones manually, but not that much.
Also, I've fixed my script to output duplicate globals. It's a nice new feature.
I edited my post a few minutes ago, so you missed it, sorry.

Still missing: some support for create_function stuff and nested functions... (Yes, they're not taken into account.)
Quote
(is there some styling in the table tag in wedge btw? I added a few basic color schemes in Protendo,in  a new table-like tag with headers and title as well, very useful for me at least :D ).
No, I don't think we changed anything to that tag. You're welcome to share suggestions, though.
Quote
Having unused globals takes memory, right? Thats the main point in removing them?
I don't know. Really, I don't think there's much of a point in worrying about that generally, but I'd recommend removing them from files that are loaded on every page load by SMF/Elk/Wedge/Pro/etc, because less is better.
Quote
oh, and I added some to the script to read the results better(for me) so it now just reads:
Oh, I didn't style anything, because on that script, I'll simply right-click, view source, and Opera 19 gives me line numbers with it, allowing me to know how many changes I have yet to make, plus a monospace font, which is more readable for code, of course.
I'll leave it up to you, though!
Re: A short script to remove unused, useless globals in PHP.
« Reply #5, on November 16th, 2013, 12:30 PM »
Quote from Bloc on November 16th, 2013, 12:02 PM
Come to think of it - I am illiterate at this really - the use of objects, they also take up memory..so could Elkarte with so few globals in fact be using just as much memory as SMF due to them? Maybe some benchmark/resource meter for PHP will show this, I need to investigate further.
Objects take a bit more memory, more processing power, and are generally not the way to go if you're looking into performance.
They are, however, very much welcome for complex code. And when it comes to micro-optimization, objects are not really worth it. I made many tests on the system object, for instance. Replacing $user_info['something'] (+ global) with we::$user['something'] (no global) had a tiny, tiny performance impact, but not enough to justify not using it.
In case it doesn't show... I hate globals :lol:

Re: A short script to remove unused, useless globals in PHP.
« Reply #6, on November 16th, 2013, 12:49 PM »
Ah, ok..still, its worthwhile setting the globals correctly, so I'll hunt down some of them. It may take a while before I reach all 2000 hah. It will be  more tidy code though, removing those unused, good when hunting down bugs too.

OOP yes, I've been reluctant to adopt to them, but see their use of course. Its that though, beginning to use them makes me think how the whole system may be better using it, and that implies a total rewrite - which is very time-consuming. I simply don't have the time for it, and fear my theme/feature ideas will wither away while doing it... :P :)

About table tags..this is what I put on Protendo, its a simpler code and it certainly need some error checking(its now up to the author to do it correctly, I have an idea of a popup where you just fill in items though, and its inserted with the right tags for you), but my goal was "simplify" foremost. Not everyone understands table/tr/td so well. :)

http://protendo.net/index.php?topic=9.0


TE

  • Posts: 286
Re: A short script to remove unused, useless globals in PHP.
« Reply #7, on November 16th, 2013, 06:52 PM »Last edited on November 16th, 2013, 07:07 PM
yay. Thank you for that little helper.

Found some real ones in Elk with it, but some are definetely false reports.. I'm currently down to a total of 21 for Elk :whistle: 8 from upgrade.php, but there's the eval for the parsed SQL code.

Make a github repo and push it, It's a nice litte tool :cool:
Posted: November 16th, 2013, 06:32 PM

checked with your updated version, 2 remaining and these are from an external library ::) Won't fix them but Elk is completely clean.

Edit: I left those in install.php and upgrade.php, too..
Thorsten "TE" Eurich - Former SMF Developer & Converters Guru

Nao

  • Dadman with a boy
  • Posts: 16,077
Re: A short script to remove unused, useless globals in PHP.
« Reply #8, on November 16th, 2013, 11:11 PM »
Quote from Bloc on November 16th, 2013, 12:49 PM
Ah, ok..still, its worthwhile setting the globals correctly, so I'll hunt down some of them. It may take a while before I reach all 2000 hah. It will be  more tidy code though, removing those unused, good when hunting down bugs too.
Yes, it's always good to have a clean codebase!
I can't believe SMF's has always been so 'dirty', in comparison... But I did spend many months fixing up all the code.
Quote from Bloc on November 16th, 2013, 12:49 PM
OOP yes, I've been reluctant to adopt to them, but see their use of course. Its that though, beginning to use them makes me think how the whole system may be better using it, and that implies a total rewrite - which is very time-consuming. I simply don't have the time for it, and fear my theme/feature ideas will wither away while doing it... :P :)
OOP can't live without at least a bit of procedural code here and there I'll say, because otherwise, might as well start using a framework like Laravel or something even newer and fancier, and then start rewriting everything on top of it, etc... (Like FluxBB is doing, for instance. Well, that version 2.0 has been in development hell for months now, so... Maybe not such a good idea.)
Quote from Bloc on November 16th, 2013, 12:49 PM
About table tags..this is what I put on Protendo, its a simpler code and it certainly need some error checking(its now up to the author to do it correctly, I have an idea of a popup where you just fill in items though, and its inserted with the right tags for you), but my goal was "simplify" foremost. Not everyone understands table/tr/td so well. :)
Very, very nice.
I could see you turning into a more familar tag family, though:
[table][th]Hello|World|!!![/th][tr]Catch|me if you|can[/tr][/table]
I'm sure we could even tweak the internal table decoding functions to support this alternative format, and then push for its use.
Re: A short script to remove unused, useless globals in PHP.
« Reply #9, on November 16th, 2013, 11:16 PM »
Quote from TE on November 16th, 2013, 06:52 PM
yay. Thank you for that little helper.
You're welcome.
Quote
Found some real ones in Elk with it, but some are definetely false reports.. I'm currently down to a total of 21 for Elk :whistle: 8 from upgrade.php, but there's the eval for the parsed SQL code.
Impressive.
Quote
Make a github repo and push it, It's a nice litte tool :cool:
I don't know, do you think it'll be that helpful..? What can be added to it? Apart from trying to fix false positives (always a question mark on whether it's possible to do much) and adding support for nested functions (yes, this one's doable), I don't know what else to do... I tried changing the code to allow for the removal of strings, but considering there are several ways to start a string in PHP, it started getting complicated, and then I gave up on that one...

Anyway, I'm not adverse to the idea of making a repo out of it; it's just that I'm not into the habit of pushing a repo that's just one file, and a few dozen lines... ;)
Quote
checked with your updated version, 2 remaining and these are from an external library ::) Won't fix them but Elk is completely clean.
Well, I'm sure you already did 90% of the work through NetBeans, didn't you? ;)
I decided against using NetBeans for that generally, because of the many false positives, such as "undeclared variable" when doing preg_match_all($regex, $string, $my_new_variable)... Yeah, NetBeans, I'm not going to initialize a variable just because you can't fathom that PHP would actually initialize the variable this way, hmm...
Quote
Edit: I left those in install.php and upgrade.php, too..
I'm sure Elk isn't 'upgradeable' via upgrade.php just like Wedge isn't, so yeah, you can probably leave it out... ;)
As for install.php, it doesn't have many issues with global, IIRC.

emanuele

  • Posts: 125

MultiformeIngegno

  • Posts: 1,337

nolsilang

  • Lurking <i class=
  • Posts: 106

TE

  • Posts: 286