Re: svn commit: r1098322 - /struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
Make Struts2 independent from a specific log implementation is a nice goal. +1 Johannes Rene Gielen wrote: There is another possible solution / improvement we might want to think of: Switch our logging system to slf4j facade logging, which addresses - besides a lot of other cool things - the parameter evaluation issue with varags and late concatenation. See: http://www.slf4j.org/ http://www.slf4j.org/faq.html - -- web: http://www.jgeppert.com twitter: http://twitter.com/jogep -- View this message in context: http://struts.1045723.n5.nabble.com/Re-svn-commit-r1098322-struts-sandbox-trunk-struts2-cdi-plugin-src-main-java-org-apache-struts2-cdi-a-tp4366559p4369135.html Sent from the Struts - Dev mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
Re: [jira] [Commented] (WW-3474) JSR 330, javax.inject implementation in Struts2
What about the caching at all ? Because internal Struts mechanism can overdo the job that is already done in Weld. Regards -- Łukasz + 48 606 323 122 http://www.lenart.org.pl/ Warszawa JUG conference - Confitura http://confitura.pl/ 2011/5/3 Rene Gielen gie...@it-neering.net: Better, of course! On 01.05.11 16:23, Lukasz Lenart wrote: What was the idea to use caching in CdiObjectFactory::getInjectionTarget() ? It can be a bit slow as it's using synchronize(this) each time (for reading and writing). I've switched into ConcurrentHashMap but maybe removing cache at all would be the best. Kind regards -- René Gielen IT-Neering.net Saarstrasse 100, 52062 Aachen, Germany Tel: +49-(0)241-4010770 Fax: +49-(0)241-4010771 Cel: +49-(0)163-2844164 http://twitter.com/rgielen - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
Re: svn commit: r1098322 - /struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
Hi,
I've removed the parts where concatenating was over constants -
constant message plus constant as a string. I'm pretty sure that the
modern JIT compilers would optimize that as well. And the code is more
readable IMHO.
Another thing, debug(), info(), etc checks if the given log level is
set up or not and then performs logging (IO request, sending e-mail,
etc). So, it will not affect performance as well (except if there is a
bug ;-) )
I agree, if we're using more complicated statements, we should
consider to use or not logging gates, but it shouldn't be a common
pattern. Maybe less logging would be better instead.
I'm thinking to remove some other parts, like this for example:
LOG.info([findBeanManager]: Checking for BeanManager under JNDI key
+ CDI_JNDIKEY_BEANMANAGER_COMP);
It's just a useless information, by spec (CDI and Weld), BM have to be
under two keys and if we've checked both and didn't find it, we should
log message like this:
LOG.error(Could not get BeanManager from JNDI context, checked under
[+CDI_JNDIKEY_BEANMANAGER_COMP+] and
[+CDI_JNDIKEY_BEANMANAGER_APP+], e);
From a user perspective this's the most informative message.
Performance shouldn't be the only factor here.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
Warszawa JUG conference - Confitura http://confitura.pl/
2011/5/3 Rene Gielen gie...@it-neering.net:
Lukasz,
why were you removing the logging gates, aka the if blocks around log
statements? I agree that for simple String parameters (without
concatenation) they might be left out, but for everything else they
really help a lot for performance - that's why I use them in general.
We should even consider reviewing S2 code if there are more unguarded
log statements, to squeeze out the last bit of performance :)
See also http://logging.apache.org/log4j/1.2/faq.html#a2.3
- René
On 01.05.11 16:29, lukaszlen...@apache.org wrote:
Author: lukaszlenart
Date: Sun May 1 14:29:02 2011
New Revision: 1098322
URL: http://svn.apache.org/viewvc?rev=1098322view=rev
Log:
Removes unnecessary checking for log level and uses ConcurrentHashMap
instead of HashMap
Modified:
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
Modified:
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
URL:
http://svn.apache.org/viewvc/struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java?rev=1098322r1=1098321r2=1098322view=diff
==
---
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
(original)
+++
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
Sun May 1 14:29:02 2011
@@ -23,14 +23,14 @@ import com.opensymphony.xwork2.ObjectFac
import com.opensymphony.xwork2.util.logging.Logger;
import com.opensymphony.xwork2.util.logging.LoggerFactory;
+import javax.enterprise.context.spi.CreationalContext;
import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.InjectionTarget;
-import javax.enterprise.context.spi.CreationalContext;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
import java.util.Map;
-import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
/**
* CdiObjectFactory allows Struts 2 managed objects, like Actions,
Interceptors or Results, to be injected by a Contexts
@@ -52,7 +52,7 @@ public class CdiObjectFactory extends Ob
protected BeanManager beanManager;
protected CreationalContext ctx;
- MapClass?, InjectionTarget? injectionTargetCache = new
HashMapClass?, InjectionTarget?();
+ MapClass?, InjectionTarget? injectionTargetCache = new
ConcurrentHashMapClass?, InjectionTarget?();
public CdiObjectFactory() {
super();
@@ -76,25 +76,17 @@ public class CdiObjectFactory extends Ob
BeanManager bm;
try {
Context initialContext = new InitialContext();
- if (LOG.isInfoEnabled()) {
- LOG.info([findBeanManager]: Checking for BeanManager under
JNDI key + CDI_JNDIKEY_BEANMANAGER_COMP);
- }
+ LOG.info([findBeanManager]: Checking for BeanManager under
JNDI key + CDI_JNDIKEY_BEANMANAGER_COMP);
try {
bm = (BeanManager)
initialContext.lookup(CdiObjectFactory.CDI_JNDIKEY_BEANMANAGER_COMP);
- } catch ( NamingException e ) {
- if (LOG.isWarnEnabled()) {
- LOG.warn([findBeanManager]: Lookup failed.);
- }
- if (LOG.isInfoEnabled()) {
- LOG.info([findBeanManager]: Checking for BeanManager
under JNDI key + CDI_JNDIKEY_BEANMANAGER_APP);
- }
+
Re: svn commit: r1098322 - /struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
2011/5/4 Johannes Geppert jo...@apache.org: Make Struts2 independent from a specific log implementation is a nice goal. We can simply extend current Logger class and add varargs versions of the logging methods and the old ones mark as @Deprecated. Kind regards -- Łukasz + 48 606 323 122 http://www.lenart.org.pl/ Warszawa JUG conference - Confitura http://confitura.pl/ - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
Re: svn commit: r1098322 - /struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
On Wed, May 4, 2011 at 9:32 PM, Lukasz Lenart
lukasz.len...@googlemail.com wrote:
Hi,
I've removed the parts where concatenating was over constants -
constant message plus constant as a string. I'm pretty sure that the
modern JIT compilers would optimize that as well. And the code is more
readable IMHO.
Another thing, debug(), info(), etc checks if the given log level is
set up or not and then performs logging (IO request, sending e-mail,
etc). So, it will not affect performance as well (except if there is a
bug ;-) )
Not true. In order to call the method in the first place, all of the
arguments must be evaluated.
If you do this, the expensive function will _always_ be invoked,
whether 'info' level logging is enabled or not:
log.info(And the answer is: + doSomethingExpensive());
However, if you guard it, like this, the expensive function is _only_
evaluated when the guard is passed:
if (log.isInfoEnabled()) {
log.info(And the answer is: + doSomethingExpensive());
}
The usual argument I've seen for getting rid of the guards is that
most of the calls don't do anything expensive, so there's little
reason to guard them. However, it's far too easy for someone else to
come along later and modify the log message without thinking about it
perhaps now needing a guard, because it's now more expensive than it
used to be. Better, then, to always guard rather than kill performance
later by accident.
--
Martin Cooper
I agree, if we're using more complicated statements, we should
consider to use or not logging gates, but it shouldn't be a common
pattern. Maybe less logging would be better instead.
I'm thinking to remove some other parts, like this for example:
LOG.info([findBeanManager]: Checking for BeanManager under JNDI key
+ CDI_JNDIKEY_BEANMANAGER_COMP);
It's just a useless information, by spec (CDI and Weld), BM have to be
under two keys and if we've checked both and didn't find it, we should
log message like this:
LOG.error(Could not get BeanManager from JNDI context, checked under
[+CDI_JNDIKEY_BEANMANAGER_COMP+] and
[+CDI_JNDIKEY_BEANMANAGER_APP+], e);
From a user perspective this's the most informative message.
Performance shouldn't be the only factor here.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
Warszawa JUG conference - Confitura http://confitura.pl/
2011/5/3 Rene Gielen gie...@it-neering.net:
Lukasz,
why were you removing the logging gates, aka the if blocks around log
statements? I agree that for simple String parameters (without
concatenation) they might be left out, but for everything else they
really help a lot for performance - that's why I use them in general.
We should even consider reviewing S2 code if there are more unguarded
log statements, to squeeze out the last bit of performance :)
See also http://logging.apache.org/log4j/1.2/faq.html#a2.3
- René
On 01.05.11 16:29, lukaszlen...@apache.org wrote:
Author: lukaszlenart
Date: Sun May 1 14:29:02 2011
New Revision: 1098322
URL: http://svn.apache.org/viewvc?rev=1098322view=rev
Log:
Removes unnecessary checking for log level and uses ConcurrentHashMap
instead of HashMap
Modified:
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
Modified:
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
URL:
http://svn.apache.org/viewvc/struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java?rev=1098322r1=1098321r2=1098322view=diff
==
---
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
(original)
+++
struts/sandbox/trunk/struts2-cdi-plugin/src/main/java/org/apache/struts2/cdi/CdiObjectFactory.java
Sun May 1 14:29:02 2011
@@ -23,14 +23,14 @@ import com.opensymphony.xwork2.ObjectFac
import com.opensymphony.xwork2.util.logging.Logger;
import com.opensymphony.xwork2.util.logging.LoggerFactory;
+import javax.enterprise.context.spi.CreationalContext;
import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.InjectionTarget;
-import javax.enterprise.context.spi.CreationalContext;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
import java.util.Map;
-import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
/**
* CdiObjectFactory allows Struts 2 managed objects, like Actions,
Interceptors or Results, to be injected by a Contexts
@@ -52,7 +52,7 @@ public class CdiObjectFactory extends Ob
protected BeanManager beanManager;
protected CreationalContext ctx;
- MapClass?, InjectionTarget? injectionTargetCache = new
HashMapClass?, InjectionTarget?();
+ MapClass?, InjectionTarget? injectionTargetCache = new
Re: [VOTE][CLOSED] Release Struts 2.2.3
Great The Vote passed and the release will be graduated as GA with: +3 binding +2 not binding Thank you everyone for you time and kind regards -- Łukasz + 48 606 323 122 http://www.lenart.org.pl/ Warszawa JUG conference - Confitura http://confitura.pl/ 2011/5/3 Rainer Hermanns herma...@aixcept.de: The things I could test with my running S2 apps work fine. Therefore, +1 GA (binding) cheers, Rainer Am 27.04.2011 um 22:29 schrieb Rene Gielen: I would have wanted to spend a little more time on testing it, but from what I can say it looks good, and Matt's performance issue seems to be non-blocking - nevertheless we should keep an eye on it. Hopefully more PMC members find some time to test the build the next couple of days? +1 GA (binding) - René On 08.04.11 12:23, Lukasz Lenart wrote: The Struts 2.2.3 test build is now available. Release notes: * [https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.2.3] Distribution: * [http://people.apache.org/builds/struts/2.2.3/] Maven 2 staging repository: * [https://repository.apache.org/content/repositories/orgapachestruts-069/] Once you have had a chance to review the test build, please respond with a vote on its quality: [ ] Leave at test build [ ] Alpha [ ] Beta [ ] General Availability (GA) Everyone who has tested the build is invited to vote. Votes by PMC members are considered binding. A vote passes if there are at least three binding +1s and more +1s than -1s. The vote will remain open for at least 72 hours, longer upon request. A vote can be amended at any time to upgrade or downgrade the quality of the release based on future experience. If an initial vote designates the build as Beta, the release will be submitted for mirroring and announced to the user list. Once released as a public beta, subsequent quality votes on a build may be held on the user list. As always, the act of voting carries certain obligations. A binding vote not only states an opinion, but means that the voter is agreeing to help do the work Kind regards -- Łukasz + 48 606 323 122 http://www.lenart.org.pl/ Warszawa JUG conference - Confitura http://confitura.pl/ - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org -- René Gielen IT-Neering.net Saarstrasse 100, 52062 Aachen, Germany Tel: +49-(0)241-4010770 Fax: +49-(0)241-4010771 Cel: +49-(0)163-2844164 http://twitter.com/rgielen - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org Rainer Hermanns aixcept Willibrordstraße 82 52134 Herzogenrath - Germany w: http://aixcept.de/ t: +49 - 2406 - 979 22 11 f: +49 - 2406 - 979 22 13 m: +49 - 170 - 343 29 12 - To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
Fwd: Nexus: Promotion Completed.
Hi, I will wait 24h before updating the site Kind regards -- Łukasz + 48 606 323 122 http://www.lenart.org.pl/ Warszawa JUG conference - Confitura http://confitura.pl/ -- Forwarded message -- From: Nexus Repository Manager ne...@repository.apache.org Date: 2011/5/5 Subject: Nexus: Promotion Completed. To: Lukasz Lenart lukasz.len...@gmail.com Description: Vote passed http://markmail.org/thread/xyd4vmvnzujccslp Details: The following artifacts have been promoted to the Releases repository. archetype-catalog.xml struts2-embeddedjsp-plugin-2.2.3.pom.asc struts2-embeddedjsp-plugin-2.2.3-sources.jar.asc struts2-embeddedjsp-plugin-2.2.3.pom struts2-embeddedjsp-plugin-2.2.3-javadoc.jar.asc struts2-embeddedjsp-plugin-2.2.3.jar struts2-embeddedjsp-plugin-2.2.3-javadoc.jar struts2-embeddedjsp-plugin-2.2.3.jar.asc struts2-embeddedjsp-plugin-2.2.3-sources.jar struts2-convention-plugin-2.2.3.pom struts2-convention-plugin-2.2.3-javadoc.jar.asc struts2-convention-plugin-2.2.3.jar struts2-convention-plugin-2.2.3.pom.asc struts2-convention-plugin-2.2.3-sources.jar struts2-convention-plugin-2.2.3.jar.asc struts2-convention-plugin-2.2.3-sources.jar.asc struts2-convention-plugin-2.2.3-javadoc.jar struts2-javatemplates-plugin-2.2.3.jar.asc struts2-javatemplates-plugin-2.2.3-sources.jar struts2-javatemplates-plugin-2.2.3.pom.asc struts2-javatemplates-plugin-2.2.3-javadoc.jar.asc struts2-javatemplates-plugin-2.2.3.pom struts2-javatemplates-plugin-2.2.3-javadoc.jar struts2-javatemplates-plugin-2.2.3-sources.jar.asc struts2-javatemplates-plugin-2.2.3.jar struts2-core-2.2.3.jar.asc struts2-core-2.2.3-sources.jar struts2-core-2.2.3-javadoc.jar.asc struts2-core-2.2.3.jar struts2-core-2.2.3-sources.jar.asc struts2-core-2.2.3.pom struts2-core-2.2.3-javadoc.jar struts2-core-2.2.3.pom.asc struts2-json-plugin-2.2.3.jar.asc struts2-json-plugin-2.2.3.jar struts2-json-plugin-2.2.3-javadoc.jar.asc struts2-json-plugin-2.2.3-javadoc.jar struts2-json-plugin-2.2.3-sources.jar struts2-json-plugin-2.2.3.pom.asc struts2-json-plugin-2.2.3-sources.jar.asc struts2-json-plugin-2.2.3.pom struts2-mailreader-2.2.3.war struts2-mailreader-2.2.3-javadoc.jar struts2-mailreader-2.2.3.war.asc struts2-mailreader-2.2.3-sources.jar.asc struts2-mailreader-2.2.3-sources.jar struts2-mailreader-2.2.3-javadoc.jar.asc struts2-mailreader-2.2.3.pom struts2-mailreader-2.2.3.pom.asc struts2-jfreechart-plugin-2.2.3-sources.jar.asc struts2-jfreechart-plugin-2.2.3.jar struts2-jfreechart-plugin-2.2.3.pom struts2-jfreechart-plugin-2.2.3-sources.jar struts2-jfreechart-plugin-2.2.3-javadoc.jar struts2-jfreechart-plugin-2.2.3-javadoc.jar.asc struts2-jfreechart-plugin-2.2.3.pom.asc struts2-jfreechart-plugin-2.2.3.jar.asc struts2-oval-plugin-2.2.3.pom.asc struts2-oval-plugin-2.2.3-javadoc.jar struts2-oval-plugin-2.2.3.pom struts2-oval-plugin-2.2.3.jar.asc struts2-oval-plugin-2.2.3.jar struts2-oval-plugin-2.2.3-sources.jar.asc struts2-oval-plugin-2.2.3-sources.jar struts2-oval-plugin-2.2.3-javadoc.jar.asc xwork-core-2.2.3-sources.jar xwork-core-2.2.3.jar xwork-core-2.2.3.pom.asc xwork-core-2.2.3-sources.jar.asc xwork-core-2.2.3-javadoc.jar xwork-core-2.2.3-javadoc.jar.asc xwork-core-2.2.3.pom xwork-core-2.2.3.jar.asc struts2-junit-plugin-2.2.3-sources.jar.asc struts2-junit-plugin-2.2.3.jar.asc struts2-junit-plugin-2.2.3.pom.asc struts2-junit-plugin-2.2.3-javadoc.jar.asc struts2-junit-plugin-2.2.3.jar struts2-junit-plugin-2.2.3-javadoc.jar struts2-junit-plugin-2.2.3.pom struts2-junit-plugin-2.2.3-sources.jar struts2-osgi-admin-bundle-2.2.3.jar struts2-osgi-admin-bundle-2.2.3-sources.jar.asc struts2-osgi-admin-bundle-2.2.3.jar.asc struts2-osgi-admin-bundle-2.2.3-javadoc.jar.asc struts2-osgi-admin-bundle-2.2.3.pom struts2-osgi-admin-bundle-2.2.3.pom.asc struts2-osgi-admin-bundle-2.2.3-javadoc.jar struts2-osgi-admin-bundle-2.2.3-sources.jar struts2-portlet-plugin-2.2.3-sources.jar.asc struts2-portlet-plugin-2.2.3.jar.asc struts2-portlet-plugin-2.2.3.pom struts2-portlet-plugin-2.2.3-javadoc.jar struts2-portlet-plugin-2.2.3.pom.asc struts2-portlet-plugin-2.2.3-javadoc.jar.asc struts2-portlet-plugin-2.2.3.jar struts2-portlet-plugin-2.2.3-sources.jar struts2-gxp-plugin-2.2.3-javadoc.jar struts2-gxp-plugin-2.2.3-javadoc.jar.asc struts2-gxp-plugin-2.2.3.pom.asc struts2-gxp-plugin-2.2.3-sources.jar struts2-gxp-plugin-2.2.3.jar struts2-gxp-plugin-2.2.3.pom struts2-gxp-plugin-2.2.3.jar.asc struts2-gxp-plugin-2.2.3-sources.jar.asc struts2-config-browser-plugin-2.2.3.jar struts2-config-browser-plugin-2.2.3.jar.asc struts2-config-browser-plugin-2.2.3-sources.jar struts2-config-browser-plugin-2.2.3.pom struts2-config-browser-plugin-2.2.3-sources.jar.asc struts2-config-browser-plugin-2.2.3-javadoc.jar.asc struts2-config-browser-plugin-2.2.3.pom.asc struts2-config-browser-plugin-2.2.3-javadoc.jar struts2-archetype-convention-2.2.3-sources.jar.asc struts2-archetype-convention-2.2.3.pom struts2-archetype-convention-2.2.3.jar struts2-archetype-convention-2.2.3-sources.jar
