> 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 66 > > <https://reviews.apache.org/r/7122/diff/1/?file=155595#file155595line66> > > > > Is this what came from the closure code? Can you add some javadoc to > > make that clear. > > > > Maybe also make this protected or have a protected getter so people > > implementing their own JsCompiler can control the stack size more easily.
This is what come from closure code. I am actually will just move this to shindig.properties to make it easier to configure. > 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? 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. - Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7122/#review11577 ----------------------------------------------------------- On Sept. 14, 2012, 11:33 p.m., Henry Saputra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7122/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2012, 11:33 p.m.) > > > Review request for shindig, Ryan Baxter and Dan Dumont. > > > 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 > >
