Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow execution of phpcbf #2

Closed
sharmashivanand opened this issue Jan 4, 2018 · 10 comments
Closed

Slow execution of phpcbf #2

sharmashivanand opened this issue Jan 4, 2018 · 10 comments

Comments

@sharmashivanand
Copy link

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
Copy link
Author

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
Copy link
Author

sharmashivanand commented Jan 4, 2018

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
Copy link
Author

@sharmashivanand
Copy link
Author

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
Copy link
Owner

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

@soderlind soderlind changed the title Doesn't work Slow execution of phpcbf Jan 4, 2018
@soderlind
Copy link
Owner

@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
Copy link
Owner

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

@sharmashivanand
Copy link
Author

sharmashivanand commented Jan 4, 2018

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
Copy link
Owner

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
Copy link
Owner

Fixed in 0.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants