eclipse-ee4j / glassfish

Eclipse GlassFish
https://eclipse-ee4j.github.io/glassfish/
378 stars 143 forks source link

3.1.1 deployment performance - ejb container module get loaded for a pure web application #17038

Closed glassfishrobot closed 11 years ago

glassfishrobot commented 13 years ago

Using the felix console to monitor which module get loaded for the web application deployment, noticed the ejb container module got active and the ejb security module got resolved

208|Active | 1|org.glassfish.ejb.ejb-container (3.1.1.SNAPSHOT)

94|Resolved | 1|org.glassfish.security.ejb.security (3.1.1.SNAPSHOT)

we need to figure out which dependency had triggered the ejb related modules get loaded/resolved, they should not be for a pure web application. I will attach the test web application in the issue next.

Affected Versions

[3.1.1]

glassfishrobot commented 6 years ago
glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: please see its parent issue for the test application

glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: Further analysis showed the ejb-container jar was loaded as part of the web container initialization. And also ejb-container contains implementation classes of AnnotationHandler which will be brought in by dol module as well (SJSASFactory injects a list of AnnotationHandler classes)


Inhabitants / stack combination

Requested by LazyInhabitant-13768021(com.sun.ejb.base.io.JavaEEIOUtilsImpl, active: null) called from org.glassfish.internal.data.EngineInfo.getContainer:93 metadata

{class: [com.sun.ejb.base.io.JavaEEIOUtilsImpl] index: [com.sun.enterprise.container.common.spi.util.JavaEEIOUtils] }

Requested by ConstructorCreator-29633611(com.sun.enterprise.web.WebContainer) called from org.glassfish.internal.data.EngineInfo.getContainer:93 metadata

{class: [com.sun.enterprise.web.WebContainer] org.glassfish.api.container.Container: [com.sun.enterprise.web.WebContainer] index: [org.glassfish.api.container.Container:com.sun.enterprise.web.WebContainer] }

glassfishrobot commented 13 years ago

@glassfishrobot Commented mvatkina said: Hong,

As you can see, AnnotationHandler's everywhere cause corresponding modules to be loaded. Unless all of them are moved to the modules with Sniffers, there is not much that our modules can do.

glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: yes, several areas have similar issue and I am looking for a uniformed solution. Moved to the connector module with sniffer is one possibility. I will keep you posted. I am keeping this issue under ejb-container for now as the code to be moved here will be ejb related.

glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: Aside from the AnnotationHandler related issues, let's look at the first one (current one) to bring in the ejb-container module.

According to the HK2 traces (attached in the issue), the ejb-container is brought in as part of the web container initialization. And further look at the WebContainer code, there is this injection: @Inject private JavaEEIOUtils javaEEIOUtils

This injection loads its impl class JavaEEIOUtilsImpl in the ejb-container module (ejb/ejb-container/src/main/java/com/sun/ejb/base/io/JavaEEIOUtilsImpl.java) which makes the ejb-container module loaded.

Assign to web container for initial evaluation on this issue, to see why the web container needs to use this class, and then how we can resolve the issue (maybe moving the JavaEEIOUtilsImpl.java to one of the common modules like container-common or common-utils).

Once we get this dependency removed, we can look at the annotation handlers related things.

glassfishrobot commented 13 years ago

@glassfishrobot Commented @shingwaichan said: Web container uses JavaEEIOUtils in order to serialize and deserialize the EJB references in HttpSession correctly. Assign to ejb container to see whether this implementation can be moved to a common module.

glassfishrobot commented 13 years ago

@glassfishrobot Commented mvatkina said: There are 2 issues reported here:

AnnotationHandlers location. This works as designed.

JavaEEIOUtils dependencies - there is already an RFE for moving them to the common location: http://java.net/jira/browse/GLASSFISH-13973

glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: Marina: 1. For annotation handlers issue, even though it's expected behavior with the current HK2 mechanism to find the impl classes from various modules. But this does not mean we should load the ejb-container module because of this. We need to move these handlers classes to another module to avoid loading ejb-container module as part of the web deployment. 2. For JavaEEIOUtils.java, I understand this is already a RFE filed it, but as we are now really looking for performance gain, we should continue looking into this: a. can we verify the dynamic injection indeed does not work? b. what about my suggestion of moving the relevant classes to ejb-connector module?

As the performance is the release priority now, I am re-opening the issue now to request further investigation. Thanks for your understanding.

glassfishrobot commented 13 years ago

@glassfishrobot Commented mvatkina said: Hong,

As the investigation for the Annotation handlers needs to be done at the GF level, I'll reassign this bug to you. When the decision is made on what is the approach to be taken GF-wide, feel free to reassign it back to us.

glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: Marina: I had discussed with Jerome yesterday. Between the two options of moving the classes back to DOL versus creating a new glue module to hold the classes, he prefers to have a new module.

glassfishrobot commented 13 years ago

@glassfishrobot Commented mvatkina said: Hong,

Did you discuss this at asarch?

glassfishrobot commented 13 years ago

@glassfishrobot Commented @honghzzhang said: I am not sure that this change requires asarch discussion, we should just not load ejb-container module as part of the web deployment, right? But if you think it's necessary, you can start a thread with asarch and Cc me. Thanks.

glassfishrobot commented 13 years ago

@glassfishrobot Commented mvatkina said: According to Scott it's no longer critical

glassfishrobot commented 13 years ago

@glassfishrobot Commented cf126330 said: committed Rev: 48926 to 3.1.2 branch, moving annotation handler classes, archivist class, scanner class, and the related from ejb-container to ejb-connector.

committed revision 48934 to trunk.

glassfishrobot commented 13 years ago

@glassfishrobot Commented cf126330 said: With the above fix and when web container obtains JavaEEIOUtils lazily, or when web container uses a generic JavaEEIOUtils impl, ejb container will not be loaded when deploying servlet/jsp-based web app.

For jsf-based app, the lookup by jsf impl of "clientStateSavingPassword" will trigger the loading of global NamedNamingProxy, which includes EJBContext. As a result, ejb-container will be loaded. This naming proxy bootstrapping happens prior to the first lookup operation, and in this case jsf happens to perform the first lookup.

glassfishrobot commented 12 years ago

@glassfishrobot Commented cf126330 said: In 4.0 trunk only, ejb-related annotation handler classes are moved back to ejb-container. See issue http://java.net/jira/browse/GLASSFISH-17988 (move ejb annotation handlers out of ejb-connector)

glassfishrobot commented 11 years ago

@glassfishrobot Commented @honghzzhang said: I just checked this for 3.1.2.2 and ejb container is no longer loaded (in active state) after deployment.

glassfishrobot commented 13 years ago

@glassfishrobot Commented File: started-208.log Attached By: @honghzzhang

glassfishrobot commented 13 years ago

@glassfishrobot Commented Issue-Links: blocks GLASSFISH-16460 is related to GLASSFISH-20028

glassfishrobot commented 13 years ago

@glassfishrobot Commented Was assigned to mvatkina

glassfishrobot commented 7 years ago

@glassfishrobot Commented This issue was imported from java.net JIRA GLASSFISH-17038

glassfishrobot commented 13 years ago

@glassfishrobot Commented Reported by @honghzzhang

glassfishrobot commented 11 years ago

@glassfishrobot Commented Marked as fixed on Monday, March 25th 2013, 8:37:57 am