google-code-export / wro4j

Automatically exported from code.google.com/p/wro4j
1 stars 1 forks source link

Parallel pre processing is not enabled with maven plugin. #782

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create a maven pom file with <extraConfigFile> pointing to a properties file 
which has a parallelPreprocessing option turned on
2. Use ro.isdc.wro.maven.plugin.manager.factory.ConfigurableWroManagerFactory 
as the wroManagerFactory
3. Run jshint goal.

What is the expected output? What do you see instead?
The jshint should be ran across the files, in parallel. But they are not. This 
can be quite slow if I have a project with lots of javascript files.

What version of the product are you using? On what operating system?
1.4.7, windows.

Please provide any additional information below.

I have the following maven plugin configuration:
-------------------------------
 <plugin>
                        <groupId>ro.isdc.wro4j</groupId>
                        <artifactId>wro4j-maven-plugin</artifactId>
                        <version>1.4.7</version>
                        <executions>
                            <execution>
                                <id>test-jshint</id>
                                <phase>validate</phase>
                                <goals>
                                    <goal>jshint</goal>
                                </goals>
                                <configuration>
                                    <wroFile>${basedir}/src/main/webapp/WEB-INF/wro-jshint.xml</wroFile>
                                    <options>browser,curly,forin,immed,latedef,trailing</options>
                                    <failNever>false</failNever>
                                    <targetGroups>core-javascript</targetGroups>
                                    <extraConfigFile>${basedir}/src/test/resources/wro.properties</extraConfigFile>
                                    <wroManagerFactory>ro.isdc.wro.maven.plugin.manager.factory.ConfigurableWroManagerFactory</wroManagerFactory>
                                </configuration>
                            </execution>
                        </executions>
</plugin>
-------------------------------

and the wro.properties file:
-------------------------------
parallelPreprocessing=true
-------------------------------

and my wro-jshint.xml file:
---------------------
<?xml version="1.0" encoding="UTF-8"?>
<groups xmlns="http://www.isdc.ro/wro">
    <!-- used for maven wro4j plugin to determine which js files are subject to jshint checks -->
    <group name='core-javascript'>
        <js>/blades/**.js</js>
    </group>
</groups>
-----------------------

Although I have specified the parallelPreprocessing flag, the jshint were not 
ran in parallel. I have created a patch file which I think should fix this. 
This improves my build time from 60 secs down to 19 secs.

Original issue reported on code.google.com by AlexWib...@gmail.com on 4 Sep 2013 at 12:24

Attachments:

GoogleCodeExporter commented 9 years ago
Apology.. I'm missing this call on WroConfigurationPropertlyLoader:

        config.setResourceWatcherUpdatePeriod(valueAsLong(sourceProperties.get(ConfigConstants.resourceWatcherUpdatePeriod.name()), 0));

Original comment by AlexWib...@gmail.com on 4 Sep 2013 at 1:22

GoogleCodeExporter commented 9 years ago
Have you tried upgrading to a newer version? The latest release is 1.7.0.

Original comment by alex.obj...@gmail.com on 4 Sep 2013 at 5:22

GoogleCodeExporter commented 9 years ago
Hi,

Sort of. I checked out the 1.7.1-SNAPSHOT (assuming that if the parallel thing 
was fixed in 1.7.0, it should have been fixed in 1.7.1-SNAPSHOT too). But I got 
the same result. The resources were not processed in a Callable.

I also notice that jshint is running much much slower. In 1.4.7 without 
parallelization, jshint completes in about 60 secs. In 1.7.1-SNAPSHOT it 
completes in ~ 4 minutes.

Original comment by AlexWib...@gmail.com on 4 Sep 2013 at 6:02

GoogleCodeExporter commented 9 years ago
Could you share the project you are using for benchmarking? I would like to run 
it locally with newer and older version. The slowness of 1.7.1-SNAPSHOT can be 
explained by usage of a newer version of jshint, but it shouldn't be 4 times 
slower... 

Original comment by alex.obj...@gmail.com on 4 Sep 2013 at 7:00

GoogleCodeExporter commented 9 years ago
Hmmm.... it would be a little bit hard, as the project is for a client, and I 
dont think they will be happy if I put it here. But to give an idea, there are 
410 resources to be processed. 

I will try to switch to some other version (between 1.4.7 - 1.7.1-SNAPSHOT), 
and see at which point the problem starts to occur. Nevertheless, the parallel 
thingy doesnt work in 1.7.1-SNAPSHOT, and the patch file above seems to fix it.

Original comment by AlexWib...@gmail.com on 4 Sep 2013 at 7:04

GoogleCodeExporter commented 9 years ago
I don't need necessarily your client application javascript resources. Having a 
sample project, containing the same amount of javascript, would be useful to 
perform a benchmark. You can easily replace javascripts you don't want to share 
with something open source....

Original comment by alex.obj...@gmail.com on 4 Sep 2013 at 7:07

GoogleCodeExporter commented 9 years ago
Hi,

Here is a sample project. You can compare v 1.4.7 against 1.7.0
On my computer, running mvn clean test on 1.4.7 takes about 57 secs. Running 
the same goal on 1.7.0 takes about 2.5 mins.

Original comment by AlexWib...@gmail.com on 4 Sep 2013 at 1:02

Attachments:

GoogleCodeExporter commented 9 years ago
Indeed, the jshint plugin is running faster with 1.4.7 version when comparing 
to 1.7.0. 
I will do some profiling to understand what makes it slower. 

Original comment by alex.obj...@gmail.com on 4 Sep 2013 at 3:16

GoogleCodeExporter commented 9 years ago
Cool. Thanks a lot for that. There is still an issue with running jshint in 
parallel though. 

Original comment by AlexWib...@gmail.com on 4 Sep 2013 at 11:35

GoogleCodeExporter commented 9 years ago
Sample project.

Original comment by AlexWib...@gmail.com on 4 Sep 2013 at 11:56

Attachments:

GoogleCodeExporter commented 9 years ago
It is already fixed in the banch called issue782.

Original comment by alex.obj...@gmail.com on 5 Sep 2013 at 5:26

GoogleCodeExporter commented 9 years ago
An update regarding jshint performance issue with 1.7.0:

The wro4j 1.7.0 uses jshint-2.1.3 (loaded from a webjar), while earlier 
versions of wro4j uses a much older jshint (prior to 2.0.0 release). Apparently 
the newer version of jshint are slower (probably because more complex static 
code analysis is being performed in newer version) comparing to older one. 

To prove that, I have changed the jshint webjar version from 2.1.3 (latest) to 
12 (older) and noticed that for the same project, processing was about ~2 times 
faster.

Original comment by alex.obj...@gmail.com on 5 Sep 2013 at 1:34

GoogleCodeExporter commented 9 years ago
One more aspect related to newer versions of jshint (post 2.0.0): these are 
using AMD with require.js. Though I'm not sure, I suspect that this has also an 
impact on performance. Unfortunately, I don't have tools which would help me to 
profile the javascript code performance and I believe this should be handled by 
jshint project (assuming they are aware about this problem).

Original comment by alex.obj...@gmail.com on 5 Sep 2013 at 1:39

GoogleCodeExporter commented 9 years ago
One more update:

I did a benchmark and noticed the following:

linting the same javascript with older version of jshint take about 160ms, 
while with jshint-2.1.3 it takes ~400ms..

That explains why processing of 180 resources is 2.5x slower with wro4j-1.7.0.

The good news is that you'll be able to run the pre processors (and all groups) 
in parallel starting with next release.

Original comment by alex.obj...@gmail.com on 6 Sep 2013 at 9:41

GoogleCodeExporter commented 9 years ago
Fixed parallelPreprocessing issue in branch 1.7.x

Original comment by alex.obj...@gmail.com on 6 Sep 2013 at 3:06

GoogleCodeExporter commented 9 years ago

Original comment by alex.obj...@gmail.com on 6 Sep 2013 at 3:07