Bug 200636 - 300ms (from 7s) speedup: Execute ModuleInstall in parallel
300ms (from 7s) speedup: Execute ModuleInstall in parallel
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Module System
7.1
Other Linux
: P1 (vote)
: 7.2
Assigned To: Jaroslav Tulach
issues@platform
: API_REVIEW_FAST, PERFORMANCE
: 211605 (view as bug list)
Depends on: 210323 209780 210706 210716
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-03 13:56 UTC by Jaroslav Tulach
Modified: 2012-05-30 11:06 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Parallelism via branding (8.59 KB, patch)
2011-08-03 13:56 UTC, Jaroslav Tulach
Details | Diff
Using @ServiceProvider(path="...", service=Runnable.class) (47.61 KB, patch)
2012-03-13 10:51 UTC, Jaroslav Tulach
Details | Diff
@OnStart, @OnStop and @OnShowing (75.14 KB, patch)
2012-03-25 03:02 UTC, Jaroslav Tulach
Details | Diff
Changes in modules to use the new API (without incremented dependencies) (57.71 KB, patch)
2012-03-28 13:25 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Tulach 2011-08-03 13:56:50 UTC
Created attachment 109769 [details]
Parallelism via branding

While investigating various options to speed the execution of NetBeans up, I followed Tomáš's Hůrka idea to initialize the system in parallel. I am attaching prototype. 

The simplest way of parallelism seems to gain 4-5% on cold start on my dual core intel notebook.
Comment 1 Jesse Glick 2011-08-03 16:57:09 UTC
[JG01] I would expect the base (no parallelism) value of PARALLEL_MODULE_INSTALL to be 1, not 0.


[JG02] Is there some reason to set the NB IDE value to 16, when few people have >4 cores? Are you expecting most threads to be blocked in I/O?


[JG03] Please ensure that even when PARALLEL_MODULE_INSTALL > 1, if B depends (perhaps indirectly) on A and both have installs, it continues to be guaranteed that A's completes before B's begins. http://bits.netbeans.org/dev/javadoc/org-openide-modules/org/openide/modules/doc-files/api.html#order
Comment 2 Jaroslav Tulach 2011-08-04 12:34:45 UTC
Re. JG02: Yes, there are three threads are blocked by I/O. Thus it makes sense to run in parallel even if the number of cores is not as high. I admit 16 is an arbitrary number.

Re. JG03: I know, this is desirable. However I am afraid that computing the information will slow things down. Thus let me propose new alternative. Instead of modifying contract for ModuleInstall, let's create new interface (or maybe rather an annotation):

@OnStartup public static mainMethod(...)

The new contract would directly say that all these actions may be invoked in parallel. That will likely be easier to implement that try to honor ModuleInstall contract and risk compatibility issues.
Comment 3 Jesse Glick 2011-08-04 15:23:23 UTC
(In reply to comment #2)
> I am afraid that computing the information will slow things down.

Have you checked? There are not many ModuleInstall instances; it would not seem to take long to check for module or OIDE-M-Requires dependencies between them. (The API only implies that transitive dependencies are considered for purposes of ordering but this ought to be retained just in case.) The module system already computes a topological sort of all modules, which is quite fast last I checked; this is similar in concept.

Or are you concerned about excess serialization of MI's?

> let's create new interface (or maybe rather an annotation):
> 
> @OnStartup public static mainMethod(...)

If you are going to introduce something that sort of replaces MI, then it needs to really supersede it so MI can be @Deprecated (saving me from having to implement bug #163937 and the like!). It should probably handle hooks to be run at all different points in the module lifecycle:

- pre-GUI startup, like MI.restored
- WindowManager.invokeWhenUIReady
- WarmUp
- shutdown, like MI.closing/close
- (possibly) WindowSystemListener.before/afterLoad/Save

Introducing one more such hook, with a completely different API style and no integration with the others, would just add to the confusion.

BTW annotating a void method is awkward because (in the absence of method handles or closures in Java) there is no "thing" to register in the usual way (service, *.instance, etc.), so you need to make up a file format. It also forces the annotation processor to essentially do its own type checking on the method signature. Better to define a contract interface (or reuse Runnable) and register the impl using either LayerBuilder.instanceFile or AbstractServiceProviderProcessor.
Comment 4 Jaroslav Tulach 2011-09-19 13:45:59 UTC
I'll assign to myself to not forget to work on it.
Comment 5 Jaroslav Tulach 2011-10-11 15:20:31 UTC
NetBeans 7.1 startup is now faster than 7.0, so this fix will not be needed.
Comment 6 Jaroslav Tulach 2012-03-13 10:51:55 UTC
Created attachment 116656 [details]
Using @ServiceProvider(path="...", service=Runnable.class)

Here is an alternative proposal that uses @ServiceProvider(path="...", service=Runnable.class) or Callable.class. The patch shows how the usage in 40 modules would look like.

The idea is that as soon as ModuleClassLoader is up, the Runnables registered for "Modules/Start" would be executed in parallel (while ModuleInstalls and OSGi is being initialized). The branding could still control the throughput.

As soon as WindowSystem is up, it would invoke everyone registered for "Modules/UIReady" sequentially in AWT thread.

On shutdown, all Callable<?> registered for "Modules/Stop" would be invoked first sequentially and if none of them returns Boolean.FALSE, the shutdown would proceed to parallel invocation of Runnables registered for "Modules/Stop".

I can imagine Jesse suggesting to use string constants rather plain strings. The "Modules/Start" and "Modules/Stop" could be either in openide.modules or direclty in LifecycleManager. "Modules/UIReady" would belong to WindowManager, then.

Please review, I'd like to have overall agreement before I proceed with implementation.
Comment 7 Jesse Glick 2012-03-15 00:10:49 UTC
[JG04] String constants would certainly be an improvement, but really I would suggest using specialized annotations for these things. Otherwise it is not obvious, nor statically enforced, that Modules/Start expects a Runnable while Modules/Stop expects either a Runnable and/or a Callable, or that the Callable should be of parameter type Boolean. For example:

@ModuleStart
public class Installer implements Runnable {...}
@ModuleUIReady
public class Installer implements Runnable {...}
@ModuleStop
public class Uninstaller implements Runnable {...}
@ModuleStop
public class Closer implements Callable<Boolean> {...}

This would require AbstractServiceProviderProcessor to be made genuinely public (moved to the org.openide.util.lookup package and ctor relaxed), but that would probably be helpful anyway. At least outputFilesByProcessor should probably be made static, to permit registrations using specialized processors to be freely intermixed with @ServiceProvider.


[JG05] Probably ModuleInstall should be @Deprecated as a result of this, right? So existing MI's would continue to work with the same semantics, including topological ordering and serial execution, but we would discourage anyone from using them. And its methods are either covered by the new APIs; already deprecated; uninstalled, of dubious purpose; and validate, also poorly tested and supported and probably better handled in other ways (at the UI layer, or in an installer of some kind).

If so, the apisupport.wizards template should be modified - or just removed, since the new style is pretty concise (esp. with JG04), and the main reason for the existing template is to do the work of adding the manifest.mf registration.


[JG06] Several of the closing() impls unconditionally return true - should be checked to see if they should really be close().


[JG07] BeanInfo search path example shows that we perhaps need some declarative API for registering path entries:

@BeanInfoSearchPath
package org.netbeans.modules.xml.tax.beans.beaninfo;

@PropertyEditorSearchPath
package org.netbeans.modules.xml.tax.beans.editor;

Of course could be done in a separate API review, but would be useful for this performance goal, as well as simplifying code. Naturally would be used in o.n.core too.
Comment 8 Jaroslav Tulach 2012-03-25 03:02:33 UTC
Created attachment 117207 [details]
@OnStart, @OnStop and @OnShowing

Functional changes without API attributes (tests, documentation). I'd like to polish that by end of March and integrate.
Comment 9 Jaroslav Tulach 2012-03-28 13:21:40 UTC
I've created branch
http://hg.netbeans.org/ergonomics/rev/200636-OnStartStopShowing
it contains the API change:
http://hg.netbeans.org/ergonomics/rev/24afcd77db24
as well as implementation of the API:
http://hg.netbeans.org/ergonomics/rev/cb2a81df34e1
Comment 10 Jaroslav Tulach 2012-03-28 13:25:18 UTC
Created attachment 117405 [details]
Changes in modules to use the new API (without incremented dependencies)

I also have these changes over the whole repository ready, but I am not sure whether to apply them for 7.2. Tell me by tomorrow, please. In case the answer is no, I will integrate only changes for modules reused by JDeveloper now, I leave the rest to individual teams to pick up and integrate.
Comment 11 Vladimir Voskresensky 2012-03-28 15:18:05 UTC
Jarda, so we are creating repository from 200636-OnStartStopShowing, applying attached patch and run functional tests on Studio IDE based on that bits.
Comment 12 Vladimir Voskresensky 2012-03-28 17:02:54 UTC
Jarda, we started but are not in time to prepare Studio IDE based on your prototype, so we can test it only tomorrow. Does it work for you?
Comment 13 Jaroslav Tulach 2012-03-28 17:15:36 UTC
I'd like to integrate the API over weekend. If you will be ready then I integrate the patch as well. Otherwise only the API.
Comment 14 Jesse Glick 2012-03-29 13:42:40 UTC
[JG08] There should be an annotation replacing @ServiceProvider(service=Runnable.class, path="WarmUp"). Could be broken out into a separate API review if you prefer. (Check bug #210018 - seems that sometimes a single task is run twice?!)
Comment 15 Andrew Krasny 2012-03-29 14:28:48 UTC
Attempt to start IDE with applied patch...
IDE cannot start and following exceptions occurs:



WARNING [org.netbeans.core.startup.InstalledFileLocatorImpl]: no code name base passed when looking up var/license_accepted at org.netbeans.core.startup.Main$1LicenseHandler.shouldDisplayLicense(Main.java:460)
INFO [org.netbeans.core.startup.ModuleList]
java.lang.ClassCastException: Should extend ModuleInstall: org.netbeans.modules.java.source.JBrowseModule
        at org.netbeans.core.startup.NbInstaller.prepare(NbInstaller.java:214)
Caused: org.netbeans.InvalidException: StandardModule:org.netbeans.modules.java.source jarFile: /export/home/hudson/hgrepos/ergonomics/nbbuild/netbeans/java/modules/org-netbeans-modules-java-source.jar
        at org.netbeans.core.startup.NbInstaller.prepare(NbInstaller.java:217)
        at org.netbeans.ModuleManager.enable(ModuleManager.java:1069)
        at org.netbeans.ModuleManager.enable(ModuleManager.java:985)
[catch] at org.netbeans.core.startup.ModuleList.installNew(ModuleList.java:340)
        at org.netbeans.core.startup.ModuleList.trigger(ModuleList.java:276)
        at org.netbeans.core.startup.ModuleSystem.restore(ModuleSystem.java:295)
        at org.netbeans.core.startup.Main.getModuleSystem(Main.java:169)
        at org.netbeans.core.startup.Main.start(Main.java:305)
        at org.netbeans.core.startup.TopThreadGroup.run(TopThreadGroup.java:123)
        at java.lang.Thread.run(Thread.java:619)
WARNING [org.netbeans.core.modules]
java.lang.IllegalStateException: Module StandardModule:org.netbeans.modules.java.preprocessorbridge jarFile: /export/home/hudson/hgrepos/ergonomics/nbbuild/netbeans/java/modules/org-netbeans-modules-java-preprocessorbridge.jar could not be installed but had no problems
        at org.netbeans.core.startup.NbProblemDisplayer.problemMessagesForModules(NbProblemDisplayer.java:229)
        at org.netbeans.core.startup.NbEvents.logged(NbEvents.java:188)
[catch] at org.netbeans.Events.log(Events.java:166)
        at org.netbeans.core.startup.ModuleList.installNew(ModuleList.java:346)
        at org.netbeans.core.startup.ModuleList.trigger(ModuleList.java:276)
        at org.netbeans.core.startup.ModuleSystem.restore(ModuleSystem.java:295)
        at org.netbeans.core.startup.Main.getModuleSystem(Main.java:169)
        at org.netbeans.core.startup.Main.start(Main.java:305)
        at org.netbeans.core.startup.TopThreadGroup.run(TopThreadGroup.java:123)
        at java.lang.Thread.run(Thread.java:619)
Warning - could not install some modules:
        org.netbeans.modules.java.source - org.netbeans.InvalidException: StandardModule:org.netbeans.modules.java.source jarFile: /export/home/hudson/hgrepos/ergonomics/nbbuild/netbeans/java/modules/org-netbeans-modules-java-source.jar: java.lang.ClassCastException: Should extend ModuleInstall: org.netbeans.modules.java.source.JBrowseModule
java.lang.AssertionError
        at org.netbeans.NetigsoModule.classLoaderUp(NetigsoModule.java:129)
        at org.netbeans.ModuleManager.enable(ModuleManager.java:1041)
        at org.netbeans.ModuleManager.enable(ModuleManager.java:985)
        at org.netbeans.core.startup.ModuleList.installNew(ModuleList.java:340)
        at org.netbeans.core.startup.ModuleList.installNew(ModuleList.java:350)
        at org.netbeans.core.startup.ModuleList.trigger(ModuleList.java:276)
        at org.netbeans.core.startup.ModuleSystem.restore(ModuleSystem.java:295)
        at org.netbeans.core.startup.Main.getModuleSystem(Main.java:169)
        at org.netbeans.core.startup.Main.start(Main.java:305)
        at org.netbeans.core.startup.TopThreadGroup.run(TopThreadGroup.java:123)
        at java.lang.Thread.run(Thread.java:619)
Comment 16 Jesse Glick 2012-03-29 18:17:19 UTC
Patch is incomplete; various module's manifests are left with a stale OpenIDE-Module-Install header which ought to be deleted.
Comment 17 Vladimir Voskresensky 2012-03-29 21:56:45 UTC
btw, Yarda, is this fix expects new startup time to change from 7sec -> 300ms or new time = (7 sec - 300ms)?
Comment 18 Jaroslav Tulach 2012-03-31 16:12:53 UTC
I've merged the branch as
http://hg.netbeans.org/ergonomics/rev/64b73bd514eb
I've selected just eight ModuleInstalls and converted them. We can discuss on the Tuesday performance meeting who and how shall handle the remaining ModuleInstalls.


Re. 300ms - it is just 300ms faster.
Comment 19 Quality Engineering 2012-04-04 10:10:20 UTC
Integrated into 'main-golden', will be available in build *201204040400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/64b73bd514eb
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: Merge of #200636: API, implementation and usage in modules which are shared with JDev
Comment 20 Petr Cyhelsky 2012-05-30 11:06:59 UTC
*** Bug 211605 has been marked as a duplicate of this bug. ***


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo