This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 230565 - Enable Different Run Modes For JavaEE and HTML 5 Projects
Summary: Enable Different Run Modes For JavaEE and HTML 5 Projects
Status: RESOLVED FIXED
Alias: None
Product: javaee
Classification: Unclassified
Component: Maven (show other bugs)
Version: 7.4
Hardware: Macintosh Mac OS X
: P3 normal (vote)
Assignee: Martin Janicek
URL:
Keywords: PERFORMANCE
Depends on:
Blocks:
 
Reported: 2013-05-31 08:41 UTC by abien
Modified: 2013-11-14 10:52 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Sample output of Run action with Maven project (2.39 KB, text/plain)
2013-09-06 14:23 UTC, Petr Jiricka
Details
Possible implementation (71.22 KB, patch)
2013-11-12 14:43 UTC, Martin Janicek
Details | Diff
Possible implementation nb. 2 (74.84 KB, patch)
2013-11-12 15:25 UTC, Martin Janicek
Details | Diff
Implementation nb. 3 (73.79 KB, patch)
2013-11-13 14:52 UTC, Martin Janicek
Details | Diff
Final implementation (73.19 KB, patch)
2013-11-13 15:31 UTC, Martin Janicek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description abien 2013-05-31 08:41:11 UTC
"Run" in context of JavaEE project means "Redeploy application". In HTML 5 context "open browser". Both cases should be distinguishable in a Java EE + HTML 5 hybrid. 
"Run" on a HTML file should not re-deploy the entire application. I would expect just opening the browser.
Comment 1 Vladimir Riha 2013-05-31 08:55:31 UTC
Probably belongs to Web Project? Please reassign if not.
Comment 2 Petr Jiricka 2013-06-11 15:04:28 UTC
Interesting suggestion, I came across this problem myself.
Comment 3 David Konecny 2013-06-19 03:43:23 UTC
> "Run" on a HTML file should not re-deploy the entire application.

On first run the whole application needs to be deployed to a server. Consequent runs should trigger only incremental redeployment (if supported by the application server) and that should result into deployment of changed files only (either upon Run action or earlier upon Save if project is configured that way). Such incremental redeployment is necessary but usually it is very fast.

And that seems to be the case for Java Web App - when I save HTML file I can see it automatically refreshed in the browser which means that application is up to date on server. If I try to run a different HTML file it takes "0 seconds" because there is nothing to build and nothing to incrementally redeploy and the only task done by Run action is "open the URL in browser".

Testing the same scenario on Maven Web project behaves differently hence I think this is only Maven project issue. Maven takes about 4-8 seconds upon Run an HTML file action and seems to be doing unnecessary tasks like "run unit tests" and "build WAR" - none of that should be necessary for single file execution. Could that be improved?

> I would expect just opening the browser.

That could result into problems if file (or one of its dependencies) was changed and not redeployed. I would prefer to keep doing incremental redeploy which should be instant in case of static files like HTML, CSS, JS.
Comment 4 Petr Jiricka 2013-06-19 10:04:11 UTC
> Testing the same scenario on Maven Web project behaves differently

When I tried just saving the file in Maven, even this does not work reliably for me, I filed a separate bug for 231510 for this.

As for the long time to run the project, I found the following info: http://wiki.netbeans.org/FaqCompileOnSave#Using_Compile_on_Save_in_Maven_Projects (it is linked from Compile panel in Maven project's properties). So it looks like this change is intentional. Not sure what to do about it - if we can't do anything about it, then we may need to introduce another action for opening the browser (e.g. "View", or a top-level "Open in Browser" action) - but I'd prefer if we can fix it on the Maven side.

As a workaround, I tried opening the browser using the application node under the GlassFish node, but that does not work either - I hit bug 231509.
Comment 5 Petr Jiricka 2013-08-16 13:12:51 UTC
Bug 231510 is now fixed, so what is the status of this issue? Is it still an issue? How fast is incremental deployment currently?
Comment 6 Martin Janicek 2013-08-16 13:22:36 UTC
(In reply to Petr Jiricka from comment #5)
> what is the status of this issue? Is it still an issue? How fast is incremental deployment currently?

I don't know, I didn't test it yet. I'll take a look at it next week
Comment 7 Petr Jiricka 2013-09-06 14:23:36 UTC
Created attachment 139765 [details]
Sample output of Run action with Maven project

I must say that I still do not find the the current status satisfactory. Even when there are no changes in the sources, Run (or Debug) action still takes almost 10 seconds, which is too long. See also the attached output from a run action on a smallish project - the build alone takes 7.5s, and that's not all. 

Furthermore, typically Maven build runs tests, and these are run whether or not there were changes in the code. For my project I specified skipTests=true to mitigate this, but if you don't do that, then the action will take even much longer than 7-10 seconds.

This highlights the need to have a way to launch the target browser immediately.
Comment 8 Martin Janicek 2013-11-06 13:27:53 UTC
I'm trying to imagine typical user of this. Do we want to do this only in some certain cases or all the time? I mean, should I make this behavior only if for example CoS/DoS are enabled? 

I tried Ant Web project and it seems to me that we are using only simple run there (nothing is updated on the server).

I'm not in favor of creating different action item (or at least I wasn't able to figure out some reasonable naming for both of them to be clear what they are doing).. But I guess we could add some check box to Run customizer asking user if he/she wants to rebuild application on every run (current behavior, data up-to-date, but slower) or not (will be new behavior, quite faster, but data are not up-to-date). What do you think?
Comment 9 Petr Jiricka 2013-11-07 08:55:16 UTC
> I tried Ant Web project and it seems to me that we are using 
> only simple run there (nothing is updated on the server).

Not sure what exactly you mean here - I would expect that Run action needs to make sure that the project is up to date on the server. But if if CoS/DoS is enabled, then the expected workflow with Ant is:
1. User edits and saves the file
2. CoS/DoS kicks in and updates the app on the server
3. User presses Run in order to (re)open the browser
4. Run action detects that everything is already up to date after step 2 and nothing needs to be redeployed, and this happens very fast, so the browser is opened very fast

In Maven projects though, step 4 takes much longer for some reason. Why? Also, in Maven tests are run by default after Run action. So in practical terms, using the Run action to open the browser is a much worse experience in Maven projects.

> But I guess we could add some check box to Run customizer 
> asking user if he/she wants to rebuild application on 
> every run (...) or not (will be new behavior, quite faster, 
> but data are not up-to-date). 

I think the application needs to be always up to date on the server after Run, we can not have a mode when Run action causes stale classes on the server. 

Not sure if we can make the Run action immediate in Maven projects (like it is in Ant) - including the test run consideration. If not, I don't see another way than adding some extra UI (not necessarily an action in the project popup menu; this could be e.g. a "green arrow" icon inside the browser chooser megamenu window). Although other ideas on how to solve this are welcome.
Comment 10 Petr Jiricka 2013-11-07 08:57:58 UTC
IMO this is largely a performance issue - adding the PERFORMANCE keyword.
Comment 11 Martin Janicek 2013-11-07 10:58:18 UTC
(In reply to Petr Jiricka from comment #9)
> In Maven projects though, step 4 takes much longer for some reason. Why?

First, because "mvn package" is always run and secondly because it always run redeployment. Both these steps are not required in cases if you have up-to-date data on the server.

> I think the application needs to be always up to date on the server after
> Run, we can not have a mode when Run action causes stale classes on the
> server. 
> 
> Not sure if we can make the Run action immediate in Maven projects (like it
> is in Ant)

Yes, it's possible - I already have an implementation of this, but my current logic is a little bit different.. When using Run for the first time, there is of course need for re-build and re-deploy of application. But the next Run is only opening browser URL and do not care if the classes are up-to-date on the server (of course only in case if CoS/DoS are enabled - if they are not, everything is the same as before - rebuild and redeploy are needed always). Maybe I can change it to just do the redeploy, but I need to try how fast it will be..

At the same time I added additional check box to Run customizer which enables to choose "Always perform build" ability - and in that case it's working as before (with no respect to the CoS/DoS).

So basically I would expect that typical user using CoS/DoS should have everything faster than earlier. And IF for some reason he will gets into some troubles, he always can check the check box to return to the current behavior. What do you think?
Comment 12 Martin Janicek 2013-11-12 14:43:10 UTC
Created attachment 142099 [details]
Possible implementation

Attaching patch and CCing Milos. Could you please check the changes and comment if you have some ideas/issues with the changes in Maven Projects module?

I didn't tested functionality in EAR project, but in Web projects it seems to work fine. By default it does not rebuild project on Run (which can be changed via Run customizer and check box called "Always perform build") and thus the performance is much better.

One thing that doesn't work correctly and I'm aware of that is that the IDE is reopening new window tabs after multiple Run actions. Still not sure why is it happening, but with respect to the feature freeze, I think this can wait and might be fixed later.
Comment 13 Petr Jiricka 2013-11-12 14:51:40 UTC
Thanks Martin.

> By default it does not rebuild project on Run 

So what happens if you clean the project and then press Run?
Comment 14 Martin Janicek 2013-11-12 15:08:10 UTC
(In reply to Petr Jiricka from comment #13)
> So what happens if you clean the project and then press Run?

In such case IDE will rebuild the project.

I'm going to write some notes about whole Deploy on Save behavior here [1]. Similar page already exists for CoS [2] and I think it would make sense to gather such information so the users will have a complete reference how it should behave in most usual use cases (after clean, with DoS enabled, with "Always perform build" activated etc.).


[1] http://wiki.netbeans.org/FaqDeployOnSave
[2] http://wiki.netbeans.org/FaqCompileOnSave
Comment 15 Martin Janicek 2013-11-12 15:25:18 UTC
Created attachment 142100 [details]
Possible implementation nb. 2

Sorry, previous patch was missing one class implementation (forgot to use hg add).
Comment 16 Milos Kleint 2013-11-13 07:01:53 UTC
1. please minimize the changes done to maven/nbproject/project.xml. Editing in the ui removes the comments I places to the list of friends.
2. you assume only one AlternativeExecutor per project. 
3. the AlternativeExecutor processing depends on undocumented internal property
config.getInternalProperties().get("skip.build") that you set in J2EEPrerequisitesChecker.java based on a fileobject attribute you set sometimes before hand..
4. AlternativExecutor has a way too generic kind of name (and javadoc) when it's only really used inside CoSchecker class to perform alternate COMPILE ON SAVE execution..

item no 3. is a deal breaker to me, if you need the fileobject property and/or the internal property, please internalize it within your own implementation of AlternativeExecutorImplementation
Comment 17 Martin Janicek 2013-11-13 10:21:52 UTC
(In reply to Milos Kleint from comment #16)
> 1. please minimize the changes done to maven/nbproject/project.xml. Editing
> in the ui removes the comments I places to the list of friends.

Sure, I'll do that.

> 2. you assume only one AlternativeExecutor per project. 

Hmm, not sure how you mean it. Do you want me to change AlternativeExecutor to expect more than one implementation of SPI in the project's lookup? Do not see real use case for it, that's why I done it this way.. but I have no problem to change if you want to ;)

> 3. the AlternativeExecutor processing depends on undocumented internal
> property
> config.getInternalProperties().get("skip.build") that you set in
> J2EEPrerequisitesChecker.java based on a fileobject attribute you set
> sometimes before hand..

Right, that's not correct.. thanks! I'll do that.

> 4. AlternativExecutor has a way too generic kind of name (and javadoc) when
> it's only really used inside CoSchecker class to perform alternate COMPILE
> ON SAVE execution..

Any suggestions? I don't know, currently it's used only for skipping the build phase, but I guess there is no reason to not to use it for different use cases. But if you want to see something like BuildSkipper, I'm fine with that as well ;)
Comment 18 Milos Kleint 2013-11-13 10:27:46 UTC
(In reply to Martin Janicek from comment #17)
> (In reply to Milos Kleint from comment #16)
> > 2. you assume only one AlternativeExecutor per project. 
> 
> Hmm, not sure how you mean it. Do you want me to change AlternativeExecutor
> to expect more than one implementation of SPI in the project's lookup? Do
> not see real use case for it, that's why I done it this way.. but I have no
> problem to change if you want to ;)

yes, since it's part of the API contract, I would expect it to support multiple ones. I know about the problems with project lookup items ordering, but anyway

> > 4. AlternativExecutor has a way too generic kind of name (and javadoc) when
> > it's only really used inside CoSchecker class to perform alternate COMPILE
> > ON SAVE execution..
> 
> Any suggestions? I don't know, currently it's used only for skipping the
> build phase, but I guess there is no reason to not to use it for different
> use cases. But if you want to see something like BuildSkipper, I'm fine with
> that as well ;)

CompileOnSaveAlternativeExecutor? or CoSAltExecutor?
Comment 19 Martin Janicek 2013-11-13 13:10:55 UTC
(In reply to Milos Kleint from comment #18)
> yes, since it's part of the API contract, I would expect it to support
> multiple ones. I know about the problems with project lookup items ordering,
> but anyway

One more question, what is the expected behavior if one of registered executors will not be successful and return false? Do we want to perform standard behavior or not in such case? I mean do we want to perform default behavior only if all of them fails or is it enough if one of them fails?

I would be in favor of option one (only if all of them fails), but I'm not sure about that - can't imagine real use case. What do you think?

> CompileOnSaveAlternativeExecutor? or CoSAltExecutor?

Ok, I change it to CoSAlternativeExecutor
Comment 20 Milos Kleint 2013-11-13 13:31:17 UTC
(In reply to Martin Janicek from comment #19)
> (In reply to Milos Kleint from comment #18)
> > yes, since it's part of the API contract, I would expect it to support
> > multiple ones. I know about the problems with project lookup items ordering,
> > but anyway
> 
> One more question, what is the expected behavior if one of registered
> executors will not be successful and return false? Do we want to perform
> standard behavior or not in such case? I mean do we want to perform default
> behavior only if all of them fails or is it enough if one of them fails?
> 
> I would be in favor of option one (only if all of them fails), but I'm not
> sure about that - can't imagine real use case. What do you think?
> 


by "be successful" you mean "not take over the build"? If none of the alternate executors takes over the build, the default execution proceeds. and we only proceed executors until we find one that takes over the build. The same pattern is used by the PrerequisiteCheckers themselves I think.
Comment 21 Martin Janicek 2013-11-13 14:52:35 UTC
Created attachment 142142 [details]
Implementation nb. 3

1. Only necessary changes are in maven/nbproject/project.xml now
2. AlternativeExecutor name changed to CoSAlternativeExecutor (same prefix added also for SPI class and maven.j2ee implementation)
3. CoSAlternativeExecutor assumes more than one registered SPI per project
4. Behavior around skip.build property moved to the CoSAlternativeExecutorImpl

+ few small changes (added @NonNull annotations, few JavaDoc improvements)

..feel free to comment if you will not be satisfied with ;)
Comment 22 Milos Kleint 2013-11-13 15:01:34 UTC
Comment on attachment 142142 [details]
Implementation nb. 3

overall looks ok to me, just as few nitpicks below:

there is already a package org.netbeans.modules.maven.spi.cos, which is probably better location than org.netbeans.modules.maven.spi.execute given that the new interface only applies within CoS.
and since org.netbeans.modules.maven.api.execute is not public anyway, it can be merged with org.netbeans.modules.maven.cos

diff --git a/maven.j2ee/nbproject/project.xml b/maven.j2ee/nbproject/project.xml
--- a/maven.j2ee/nbproject/project.xml
+++ b/maven.j2ee/nbproject/project.xml
@@ -263,7 +263,7 @@
                     <compile-dependency/>
                     <run-dependency>
                         <release-version>2</release-version>
-                        <specification-version>2.88</specification-version>
+                        <specification-version>2.97</specification-version>

looks weird to upgrade from 88 to 97, intentional?
Comment 23 Martin Janicek 2013-11-13 15:08:58 UTC
(In reply to Milos Kleint from comment #22)
> there is already a package org.netbeans.modules.maven.spi.cos, which is
> probably better location than org.netbeans.modules.maven.spi.execute given
> that the new interface only applies within CoS.

Sure I'll do that (didn't realize at the beginning that it's related only to CoS execution)

> and since org.netbeans.modules.maven.api.execute is not public anyway, it
> can be merged with org.netbeans.modules.maven.cos

Don't understand this and does not sound related to the issue. But maybe I'm wrong? :)

> looks weird to upgrade from 88 to 97, intentional?

Ye that's for purpose. I had to increase maven projects spec. version (last one was 2.96, after incrementation current one 2.97) and since I want to use new friend API/SPI classes inside maven.j2ee I have to depend on 2.97
Comment 24 Martin Janicek 2013-11-13 15:16:16 UTC
Ah, maybe I understand. But maven.cos is not public package and on the other hand maven.api.execute is friend public package. Do not think we should merge them.

I don't know do you want me to create new maven.api.cos with only CoSAlternativeExecutor or should I just let it in maven.api.execute (the SPI part is now in maven.spi.cos). It's up to you, I don't have strong opinion about this to be honest.
Comment 25 Milos Kleint 2013-11-13 15:20:51 UTC
(In reply to Martin Janicek from comment #24)
> Ah, maybe I understand. But maven.cos is not public package and on the other
> hand maven.api.execute is friend public package. Do not think we should
> merge them.
> 
> I don't know do you want me to create new maven.api.cos with only
> CoSAlternativeExecutor or should I just let it in maven.api.execute (the SPI
> part is now in maven.spi.cos). It's up to you, I don't have strong opinion
> about this to be honest.

I don't think it has to be an API at all. That's what I meant with my initial comment. Since the CoSAlternativeExecutor class is only used by CoSChecker it should be in the same package. Sorry for the misunderstanding
Comment 26 Martin Janicek 2013-11-13 15:27:03 UTC
(In reply to Milos Kleint from comment #25)
> I don't think it has to be an API at all. That's what I meant with my
> initial comment. Since the CoSAlternativeExecutor class is only used by
> CoSChecker it should be in the same package. Sorry for the misunderstanding

Weeeell it makes sense to me to provide complementary API to the SPI part.. but I guess we can move it to the friend API package when it will be useful also outside of the maven projects, sure. I'll do that
Comment 27 Martin Janicek 2013-11-13 15:31:21 UTC
Created attachment 142146 [details]
Final implementation

SPI class moved from maven.spi.execute to already existing maven.spi.cos
API class moved from maven.api.cos to maven.cos
Comment 28 Martin Janicek 2013-11-14 10:48:16 UTC
Fixed by change-set: web-main #c70f3c573419
Comment 29 Martin Janicek 2013-11-14 10:52:46 UTC
I created issue 238360 to track problem with reopening the output window.