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

Add option to disable/limit threads #1607

Closed
struys opened this issue Mar 3, 2016 · 13 comments
Closed

Add option to disable/limit threads #1607

struys opened this issue Mar 3, 2016 · 13 comments

Comments

@struys
Copy link

struys commented Mar 3, 2016

We updated to a more recent version of the closure compiler and we're seeing it thrash our development machines. I believe this is happening because closure is using a ton of CPU, I'd like a way to limit the CPU usage by either disabling or limiting the number of threads used.

@Dominator008
Copy link
Contributor

To my knowledge the compiler is essentially single-threaded. Can you provide more details about the thrashing you've seen? How much memory was the compiler using? Which versions did you update from and to?

@ChadKillingsworth
Copy link
Collaborator

It's true - like any workhorse compiler, closure-compiler makes my macbook pro sounds like it is getting ready for a takeoff. But that's any compiler ...

@Dominator008
Copy link
Contributor

@struys Compiler.java has a diableThreads() option but I doubt it will help.
@ChadKillingsworth I'd be more concerned if we are using a ton of memory.

@dimvar
Copy link
Contributor

dimvar commented Mar 3, 2016

@struys You can pass the flag --tracer_mode=TIMING_ONLY to the compiler to get some timing data for a compilation job, and then compare the old version to the recent version to see if anything jumps out.

Closing this issue since there is no actionable information here. If you find some slowdown for some specific pass, and you can give us instructions to reproduce, then we can look into it.

@dimvar dimvar closed this as completed Mar 3, 2016
@adityavohra7
Copy link

(I work with @struys)

Can you provide more details about the thrashing you've seen?

We're using plovr.
We see 22 concurrent plovr.jar processes, with 1013 threads totall (~44 each).

How much memory was the compiler using?

The processes are using ~150MB resident memory, each.
This leads to our build beeing oomkilled if even three are running concurrently, on a 60GB machine.

Which versions did you update from and to?

From: v20150729 To: v20151216

The TIMING_ONLY run didn't show anything notable.

adityavohra7 referenced this issue Mar 26, 2016
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=100425540
@adityavohra7
Copy link

The cause is the new Executors.newCachedThreadPool, on this line: 83c9945#diff-4d24c1568736e3d7a134144ed11286aaR46

@adityavohra7
Copy link

We plan to submit a patch for this and would like some guidance; how would you like to see this configured. The current thinking is to make the number of threads default to the number of CPU cores, and make this ratio tunable (something like --threads-per-cpu 2).

@Dominator008
Copy link
Contributor

Interesting, 44 threads per process sounds a a bit ridiculous. I thought that particular commit shouldn't cause more threads to be created. We were using Executors.newCachedThreadPool already.

@concavelenz
Copy link
Contributor

Sounds like this is a plovr issue. As stated the compiler should only need using ones thread per compilation job. Embedders are free to start as many parallel jobs are they wish / think they can handle.

@supersteves
Copy link
Contributor

I ran into a similar issue. In your case, Plovr must be starting and killing multiple compilation jobs (different Compiler instances), possibly sequentially.

Perhaps Plovr is interrupting the thread. That's what I'm trying. But Closure Compiler doesn't have a mechanism to cancel an ongoing compilation - it merely forgets the thread is running when interrupted. See CompilerExecutor.

I'm experimenting with Compiler.disableThreads() to work around this, but may consider a PR to properly interrupt compilation, some volatile boolean in Compiler which causes a commonly access method such as NodeTraversal.traverse to fail propagate some runtime exception, perhaps.

@concavelenz
Copy link
Contributor

I think to interrupt, you may just be able to report an error to the compiler, then the compiler will stop before the next pass starts unless you are running in "ide mode":

@supersteves
Copy link
Contributor

Thanks. I need something more prompt, so I extended Compiler, overrode a bunch of public/protected methods, threw a RuntimeException and caught it higher up. Works well, but there'll be some fallout when I next update the compiler version probably. I think a PR for an API to interrupt compilation is probably to specific to my own uses, so unless you say otherwise I'll leave it at that.

Also see #1822

@Dominator008
Copy link
Contributor

Coming back to this as I'm investigating something similar internally at Google. So between v20150729 and v20151216 something did change: 73eb61b
I changed the CompilerExecutor from a static variable to an instance variable. If each plovr.jar is creating a bunch of Compiler instances, each Compiler now has their own thread pool instead of sharing a common one. However, I don't think that should affect the total number of threads though. It's still unclear to me why tons of threads were created.

In an upcoming change I'm changing newCachedThreadPool to newSingleThreadExecutor anyway.

brad4d pushed a commit that referenced this issue Jul 6, 2017
Because projects might reuse the Compiler instance for multiple compilations, set a new ExecutorService at the beginning of runInCompilerThread().

Should fix #1607

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=160981960
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

7 participants