> On Sept. 15, 2012, 2:52 p.m., Stanton Sievers wrote: > > trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java, > > line 263 > > <https://reviews.apache.org/r/7122/diff/1/?file=155595#file155595line263> > > > > The javadoc for this method states "Disable threads. This is for > > clients that run on AppEngine and don't have threads." That concerns me, > > as this is certainly not the case we're in. Is the javadoc incomplete? > > Henry Saputra wrote: > Its kinda misleading bc internally the closure JS compiler doe not use > thread pool. So when compiling large number of JS resources it could spawn a > lot of threads. > > I am planning to contribute code to add thread pool support to the code > but for now we could just disable closure compiler thread and let it run in > the thread than Shindig code created to run the compilation from the thread > pool. > > Dan Dumont wrote: > The closure threads would be blocked from growing beyond the size of the > shindig thread pool though, right? We only ever compile one file with any > given compiler instance. > > In any case, I think this is a good change. No need to spawn more > threads.
Dan, that's correct. After Dan's fix before it would only grow up to the size of Shindig thread pool but it creates additional child thread per thread worker created by Shindig which is not efficient. - Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7122/#review11577 ----------------------------------------------------------- On Sept. 15, 2012, 4:58 p.m., Henry Saputra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7122/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2012, 4:58 p.m.) > > > Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. > > > Description > ------- > > Disable thread in the closure compiler code since Shindig manage the thread > in the ClosureJSCompiler class via ExecuterService framework. > Otherwise internally the closure compiler create another thread then execute > the compile in that new thread instead > > But we need to create the thread big enough stack to handle the compilation. > Copy the max stack size from the closure compiler code. > > > Diffs > ----- > > > trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java > 1384587 > > Diff: https://reviews.apache.org/r/7122/diff/ > > > Testing > ------- > > Run smoke test with common container with debug sets to false to run the > closure js compiler. > > > Thanks, > > Henry Saputra > >
