Emmanuel's optimization is interesting, but it removes the possibility of activating debugging at runtime. It's not needed often but when it's needed you'll certainly want to have it.

My 2cts ...

- Eero

Julien Vermillard wrote:
Hi,
interesting solution Emm, but you got an idea of the speedup ?
Because it's really not helping readability of code :)

Perhaps we can do that on critical parts of code.

Sangjin : does some of you recent change in 1.0 branches need to be
ported to trunk ? If so I'll try to take a look.

Julien

On Tue, 05 Aug 2008 09:10:32 +0200
Emmanuel Lecharny <[EMAIL PROTECTED]> wrote:

Sangjin Lee wrote:
Thanks for pointing that out.  It was a mechanical
search-and-replace.  I also wasn't familiar with the capability of
SLF4J in terms of evaluating args.  I'll look at them again, and
remove isXXXEnabled() calls.
You can keep the isXXXEnabled. It should not make a big difference.
But Julien is right, when using the {} to pass arguments to the
XXX.log( message, arg1...) the cost is almost the same than simply
doing a ifXXXEnabled() {...}.

The ultimate optimization, considering that the logs will become
static, would be to add a static with an evaluation of the log state :

private static final Logger LOG = LoggerFactory.getLogger( <your class>.class );

    /**
     * Speedup for logs
     */
    private static final boolean IS_DEBUG = LOG.isDebugEnabled();
...
    if ( IS_DEBUG ) {
        LOG.debug( blah );
    }

Doing so will allow the JVM to simply remove the test, as if DEBUG is not true, and as it's static, the optimizer will consider that the
inner code will never be executed.

It might worth the extra effort for DEBUG, certainly not for warnings
or errors.

My 2cts ...

Regards,
Sangjin


On Mon, Aug 4, 2008 at 11:23 PM, Julien Vermillard
<[EMAIL PROTECTED]>wrote:

Why using isTraceEnabled here ?
there is no concatenation and even if there was, we could use {}
in slf4j call.
if (LOG.isTraceEnabled()) {
    LOG.trace("enter
BasicScheme.authenticate(UsernamePasswordCredentials, String)");
}

Julien

2008/8/4  <[EMAIL PROTECTED]>:
Author: sjlee
Date: Mon Aug  4 12:46:17 2008
New Revision: 682480

URL: http://svn.apache.org/viewvc?rev=682480&view=rev
Log:
ASYNCWEB-24

Wrapped debug() and trace() calls with isDebugEnabled() and
isTraceEnabled() calls.  Also prefer StringBuilder over
StringBuffer.
Modified:

 
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthScope.java
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthState.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/BasicScheme.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/DigestScheme.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTCredentials.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTLMScheme.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/UsernamePasswordCredentials.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/codec/HttpDecoder.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/EncodingUtil.java mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/ParameterFormatter.java
Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthScope.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthScope.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthScope.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthScope.java
Mon Aug  4 12:46:17 2008
@@ -265,7 +265,7 @@
     * @see java.lang.Object#toString()
     */
    public String toString() {
-        StringBuffer buffer = new StringBuffer();
+        StringBuilder buffer = new StringBuilder();
        if (this.scheme != null) {
            buffer.append(this.scheme.toUpperCase());
            buffer.append(' ');

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthState.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthState.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthState.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/AuthState.java
Mon Aug  4 12:46:17 2008
@@ -155,7 +155,7 @@
    }

    public String toString() {
-        StringBuffer buffer = new StringBuffer();
+        StringBuilder buffer = new StringBuilder();
        buffer.append("Auth state: auth requested [");
        buffer.append(this.authRequested);
        buffer.append("]; auth attempted [");

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/BasicScheme.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/BasicScheme.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/BasicScheme.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/BasicScheme.java
Mon Aug  4 12:46:17 2008
@@ -107,7 +107,9 @@
     */
    public String authenticate(Credentials credentials,
HttpRequestMessage request) throws AuthenticationException {
-        LOG.trace("enter BasicScheme.authenticate(Credentials,
HttpMethod)");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter
BasicScheme.authenticate(Credentials,
HttpMethod)");
+        }

        if (request == null) {
            throw new IllegalArgumentException("Request may not be
null");
@@ -137,7 +139,9 @@
     */
    public static String authenticate(UsernamePasswordCredentials
credentials, String charset) {
-        LOG.trace("enter
BasicScheme.authenticate(UsernamePasswordCredentials, String)");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter
BasicScheme.authenticate(UsernamePasswordCredentials, String)");
+        }

        if (credentials == null) {
            throw new IllegalArgumentException("Credentials may
not be
null");
@@ -145,7 +149,7 @@
        if (charset == null || charset.length() == 0) {
            throw new IllegalArgumentException("charset may not
be null
or empty");
        }
-        StringBuffer buffer = new StringBuffer();
+        StringBuilder buffer = new StringBuilder();
        buffer.append(credentials.getUserName());
        buffer.append(":");
        buffer.append(credentials.getPassword());

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/DigestScheme.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/DigestScheme.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/DigestScheme.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/DigestScheme.java
Mon Aug  4 12:46:17 2008
@@ -176,7 +176,9 @@
    public String authenticate(Credentials credentials,
HttpRequestMessage request)
    throws AuthenticationException {

-        LOG.trace("enter DigestScheme.authenticate(Credentials,
HttpMethod)");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter
DigestScheme.authenticate(Credentials,
HttpMethod)");
+        }

        UsernamePasswordCredentials usernamepassword = null;
        try {
@@ -187,7 +189,7 @@
                    + credentials.getClass().getName());
        }
        getParameters().put("methodname",
request.getRequestMethod());
-        StringBuffer buffer = new
StringBuffer(request.getPath());
+        StringBuilder buffer = new
StringBuilder(request.getPath()); String query =
request.getUrl().getQuery(); if (query != null) {
            if (query.indexOf("?") != 0) {
@@ -219,7 +221,9 @@
     */
    private String createDigest(final String uname, final String
pwd)
throws AuthenticationException {
-        LOG.trace("enter DigestScheme.createDigest(String,
String,
Map)");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter DigestScheme.createDigest(String,
String,
Map)");
+        }

        final String digAlg = "MD5";

@@ -257,7 +261,7 @@
        }

        // 3.2.2.2: Calculating digest
-        StringBuffer tmp = new StringBuffer(uname.length() +
realm.length() + pwd.length() + 2);
+        StringBuilder tmp = new StringBuilder(uname.length() +
realm.length() + pwd.length() + 2);
        tmp.append(uname);
        tmp.append(':');
        tmp.append(realm);
@@ -272,7 +276,7 @@
            //      ":" unq(cnonce-value)

            String
tmp2=encode(md5Helper.digest(EncodingUtil.getBytes(a1,
charset)));
-            StringBuffer tmp3 = new StringBuffer(tmp2.length() +
nonce.length() + cnonce.length() + 2);
+            StringBuilder tmp3 = new StringBuilder(tmp2.length()
+
nonce.length() + cnonce.length() + 2);
            tmp3.append(tmp2);
            tmp3.append(':');
            tmp3.append(nonce);
@@ -297,8 +301,10 @@
        // 3.2.2.1
        String serverDigestValue;
        if (qopVariant == QOP_MISSING) {
-            LOG.debug("Using null qop method");
-            StringBuffer tmp2 = new StringBuffer(md5a1.length() +
nonce.length() + md5a2.length());
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Using null qop method");
+            }
+            StringBuilder tmp2 = new
StringBuilder(md5a1.length() +
nonce.length() + md5a2.length());
            tmp2.append(md5a1);
            tmp2.append(':');
            tmp2.append(nonce);
@@ -310,7 +316,7 @@
                LOG.debug("Using qop method " + qop);
            }
            String qopOption = getQopVariantString();
-            StringBuffer tmp2 = new StringBuffer(md5a1.length() +
nonce.length()
+            StringBuilder tmp2 = new
StringBuilder(md5a1.length() +
nonce.length()
                + NC.length() + cnonce.length() +
qopOption.length() +
md5a2.length() + 5);
            tmp2.append(md5a1);
            tmp2.append(':');
@@ -343,8 +349,10 @@
    private String createDigestHeader(final String uname, final
String
digest)
        throws AuthenticationException {

-        LOG.trace("enter DigestScheme.createDigestHeader(String,
Map, "
-            + "String)");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter
DigestScheme.createDigestHeader(String,
Map, "
+                    + "String)");
+        }

        String uri = getParameter("uri");
        String realm = getParameter("realm");
@@ -372,7 +380,7 @@
            params.add(new NameValuePair("opaque", opaque));
        }

-        StringBuffer buffer = new StringBuffer();
+        StringBuilder buffer = new StringBuilder();
        for (int i = 0; i < params.size(); i++) {
            NameValuePair param = (NameValuePair) params.get(i);
            if (i > 0) {
@@ -404,7 +412,9 @@
     * @return encoded MD5, or <CODE>null</CODE> if encoding
failed */
    private static String encode(byte[] binaryData) {
-        LOG.trace("enter DigestScheme.encode(byte[])");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter DigestScheme.encode(byte[])");
+        }

        if (binaryData.length != 16) {
            return null;
@@ -429,7 +439,9 @@
     * @throws AsyncHttpClientException if MD5 algorithm is not
supported.
     */
    public static String createCnonce() {
-        LOG.trace("enter DigestScheme.createCnonce()");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter DigestScheme.createCnonce()");
+        }

        String cnonce;
        final String digAlg = "MD5";

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTCredentials.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTCredentials.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTCredentials.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTCredentials.java
Mon Aug  4 12:46:17 2008
@@ -130,7 +130,7 @@
     * @return A string represenation of this object.
     */
    public String toString() {
-        final StringBuffer sbResult = new
StringBuffer(super.toString());
+        final StringBuilder sbResult = new
StringBuilder(super.toString());
        sbResult.append("@");
        sbResult.append(this.host);

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTLMScheme.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTLMScheme.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTLMScheme.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/NTLMScheme.java
Mon Aug  4 12:46:17 2008
@@ -173,7 +173,9 @@
     */
    public String authenticate(Credentials credentials,
HttpRequestMessage request)
        throws AuthenticationException {
-        LOG.trace("enter NTLMScheme.authenticate(Credentials,
HttpMethod)");
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("enter NTLMScheme.authenticate(Credentials,
HttpMethod)");
+        }

        if (this.state == UNINITIATED) {
            throw new IllegalStateException("NTLM authentication
process
has not been initiated");
Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/UsernamePasswordCredentials.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/UsernamePasswordCredentials.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/UsernamePasswordCredentials.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/auth/UsernamePasswordCredentials.java
Mon Aug  4 12:46:17 2008
@@ -144,7 +144,7 @@
     * @return the username:password formed string
     */
    public String toString() {
-        StringBuffer result = new StringBuffer();
+        StringBuilder result = new StringBuilder();
        result.append(this.userName);
        result.append(":");
        result.append((this.password == null) ? "null" :
this.password);

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/codec/HttpDecoder.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/codec/HttpDecoder.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/codec/HttpDecoder.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/codec/HttpDecoder.java
Mon Aug  4 12:46:17 2008
@@ -248,7 +248,9 @@
     * @throws Exception if any exception occurs
     */
    public void decodeHeader(String line, HttpResponseMessage
msg) throws
Exception {
-        LOG.debug("Processing Header Line: " + line);
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Processing Header Line: " + line);
+        }
        // first, get rid of the CRLF from linear whitespace
        line = folding.matcher(line).replaceAll("$1");
        int pos = line.indexOf(":");
@@ -420,7 +422,9 @@
     * @see Cookie
     */
    public Cookie decodeCookie(String cookieStr,
HttpResponseMessage msg)
throws Exception {
-        LOG.debug("Processing Cookie Line: " + cookieStr);
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Processing Cookie Line: " + cookieStr);
+        }
        Cookie cookie = null;

        String pairs[] = cookieStr.split(";");

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/EncodingUtil.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/EncodingUtil.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/EncodingUtil.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/EncodingUtil.java
Mon Aug  4 12:46:17 2008
@@ -103,7 +103,7 @@
     */
    private static String doFormUrlEncode(NameValuePair[] pairs,
String
charset)
        throws UnsupportedEncodingException {
-        StringBuffer buf = new StringBuffer();
+        StringBuilder buf = new StringBuilder();
        for (int i = 0; i < pairs.length; i++) {
            URLCodec codec = new URLCodec();
            NameValuePair pair = pairs[i];

Modified:
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/ParameterFormatter.java
URL:
http://svn.apache.org/viewvc/mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/ParameterFormatter.java?rev=682480&r1=682479&r2=682480&view=diff
==============================================================================
---
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/ParameterFormatter.java
(original)
+++
mina/asyncweb/branches/1.0/client/src/main/java/org/apache/asyncweb/client/util/ParameterFormatter.java
Mon Aug  4 12:46:17 2008
@@ -150,7 +150,7 @@
     * potentially unsafe special characters
     */
    public static void formatValue(
-            final StringBuffer buffer, final String value,
boolean
alwaysUseQuotes) {
+            final StringBuilder buffer, final String value,
boolean
alwaysUseQuotes) {
        if (buffer == null) {
            throw new IllegalArgumentException("String buffer may
not be
null");
        }
@@ -194,7 +194,7 @@
     * @param buffer output buffer
     * @param param the parameter to be formatted
     */
-    public void format(final StringBuffer buffer, final
NameValuePair
param) {
+    public void format(final StringBuilder buffer, final
NameValuePair
param) {
        if (buffer == null) {
            throw new IllegalArgumentException("String buffer may
not be
null");
        }
@@ -219,7 +219,7 @@
     * attribute/value pair
     */
    public String format(final NameValuePair param) {
-        StringBuffer buffer = new StringBuffer();
+        StringBuilder buffer = new StringBuilder();
        format(buffer, param);
        return buffer.toString();
    }





Reply via email to