soderlind / vscode-phpcbf

PHP Code Beautifier and Fixer for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=persoderlind.vscode-phpcbf
GNU General Public License v3.0
28 stars 10 forks source link

Slow execution of phpcbf #2

Closed sharmashivanand closed 6 years ago

sharmashivanand commented 6 years ago

which phpcs /usr/local/bin/phpcs

which phpcbf /usr/local/bin/phpcbf

phpcbf -i The installed coding standards are PEAR, Zend, PSR2, MySource, Squiz, PSR1, WordPress-VIP, WordPress, WordPress-Extra, WordPress-Docs and WordPress-Core

Settings: "phpcbf.standard": "WordPress",

sharmashivanand commented 6 years ago

So it takes several minutes to update the fixed file. I'm still trying to do console.log and see which part of the extension is taking all this while. Or may be msvsc takes minutes to detect file changes and reload the file?

sharmashivanand commented 6 years ago

So I edited

let exec = cp.spawn(this.executablePath, this.getArgs(fileName));
console.log("spawned at:" + Date.now());

and

exec.on("exit", code => {
console.log("Exit status:" + code);
console.log("Finished at:" + Date.now());

Output: spawned at:1515061443180 Finished at:1515061503327

This seems like the time difference in milliseconds was 60147 which is about 60 seconds. So it works. Does it take as long on your side?

[Extension Host] 
PHPCBF RESULT SUMMARY
-----------------------------------------------------------------------------------------------
FILE                                                                           FIXED  REMAINING
-----------------------------------------------------------------------------------------------
/private/var/folders/3w/tg21cw_d3bj848kcgjzdjwdw0000gn/T/temp-kpnkghdd.php     1      1
-----------------------------------------------------------------------------------------------
A TOTAL OF 1 ERROR WERE FIXED IN 1 FILE
-----------------------------------------------------------------------------------------------

Time: 1 mins, 0.18 secs; Memory: 8Mb
sharmashivanand commented 6 years ago

Please also see: https://github.com/Glavin001/atom-beautify/issues/893 https://github.com/nodejs/node/issues/7652 https://github.com/Glavin001/atom-beautify/pull/1059

sharmashivanand commented 6 years ago

I just closed the stdin and now it does it in seconds. Hope this is the correct way.

"Time: 135ms; Memory: 8Mb"

fs.unlink(fileName, function(err) {});
        autoFixing = false;
      });
    });

    exec.stdin.end();

    exec.stdout.on("data", buffer => {
      console.log(buffer.toString());
    });
soderlind commented 6 years ago

excellent, thank you :) I'll add, test and push a new version later today (after work).

soderlind commented 6 years ago

@shivanandwp I believe I should close stdin on close, I'm a newb at writing extensions so I might be wrong:

exec.on("close", code => {
    exec.stdin.end();
});
soderlind commented 6 years ago

Forget my last comment, closing stdin just after the promise, as you suggested, makes the execution much faster.

sharmashivanand commented 6 years ago

I'm totally newbie but I can mess around. I think it still waits for stdin before it hits close. I tried your code but it still hit a minute.

I don't know if the extension is even using stdin.

I then placed the end directly after the exec declaration. This does it in Time: 132ms; Memory: 8Mb

let exec = cp.spawn(this.executablePath, this.getArgs(fileName));
exec.stdin.end();
soderlind commented 6 years ago

I've been reading up on child processes https://nodejs.org/api/child_process.html. Tried to use execFile, but it failed. I'll move exec.stdin.end(); just after cp.spawn, test a bit more and push the code to the marketplace.

soderlind commented 6 years ago

Fixed in 0.0.3