You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commons-dev@ws.apache.org by "Alexander Veit (JIRA)" <ji...@apache.org> on 2006/12/16 16:19:21 UTC
[jira] Created: (WSCOMMONS-142) Inefficient code in TextImpl and
OMTextImpl
Inefficient code in TextImpl and OMTextImpl
-------------------------------------------
Key: WSCOMMONS-142
URL: http://issues.apache.org/jira/browse/WSCOMMONS-142
Project: WS-Commons
Issue Type: Bug
Components: AXIOM
Reporter: Alexander Veit
The methods getText() and getTextAsQName() in org.apache.axiom.om.impl.dom.TextImpl and org.apache.axiom.om.impl.llom.OMTextImpl allocate duplicate buffers and perform needless array copy operations, if input data is read from an InputStream.
The attached patches should fix the problem. Note that a base64-encoding bug is also fixed for many cases (see WSCOMMONS-101).
--- snip ---
Index: D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java
===================================================================
--- D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (revision 485855)
+++ D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (working copy)
@@ -312,21 +312,13 @@
return getTextFromProperPlace();
} else {
try {
- InputStream inStream;
- inStream = this.getInputStream();
- // int x = inStream.available();
- byte[] data;
+ InputStream inStream = this.getInputStream(); // who closes this?
+ final byte[] data = new byte[1023];
StringBuffer text = new StringBuffer();
- do {
- data = new byte[1024];
- int len;
- while ((len = inStream.read(data)) > 0) {
- byte[] temp = new byte[len];
- System.arraycopy(data, 0, temp, 0, len);
- text.append(Base64.encode(temp));
- }
-
- } while (inStream.available() > 0);
+ int len;
+ while ((len = inStream.read(data)) != -1) {
+ text.append(Base64.encode(data, 0, len));
+ }
return text.toString();
} catch (Exception e) {
throw new OMException(e);
@@ -373,21 +365,13 @@
return new QName(getTextFromProperPlace());
} else {
try {
- InputStream inStream;
- inStream = this.getInputStream();
- byte[] data;
+ InputStream inStream = this.getInputStream(); // who closes this?
+ byte[] data = new byte[1023];
StringBuffer text = new StringBuffer();
- do {
- data = new byte[1024];
- int len;
- while ((len = inStream.read(data)) > 0) {
- byte[] temp = new byte[len];
- System.arraycopy(data, 0, temp, 0, len);
- text.append(Base64.encode(temp));
- }
-
- } while (inStream.available() > 0);
-
+ int len;
+ while ((len = inStream.read(data)) != -1) {
+ text.append(Base64.encode(data, 0, len));
+ }
return new QName(text.toString());
} catch (Exception e) {
throw new OMException(e);
--- snap ---
--- snip ---
Index: D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java
===================================================================
--- D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (revision 487185)
+++ D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (working copy)
@@ -228,21 +228,13 @@
} else {
try {
// TODO we have the following code duplicated in getTextAsQName
- InputStream inStream;
- inStream = this.getInputStream();
- byte[] data;
+ InputStream inStream = this.getInputStream(); // who closes this?
+ final byte[] data = new byte[1023];
StringBuffer text = new StringBuffer();
- do {
- data = new byte[1023];
- int len;
- while ((len = inStream.read(data)) > 0) {
- byte[] temp = new byte[len];
- System.arraycopy(data, 0, temp, 0, len);
- text.append(Base64.encode(temp));
- }
-
- } while (inStream.available() > 0);
-
+ int len;
+ while ((len = inStream.read(data)) != -1) {
+ text.append(Base64.encode(data, 0, len));
+ }
return text.toString();
} catch (Exception e) {
throw new OMException(e);
@@ -284,21 +276,13 @@
return new QName(getTextFromProperPlace());
} else {
try {
- InputStream inStream;
- inStream = this.getInputStream();
- byte[] data;
+ InputStream inStream = this.getInputStream(); // who closes this?
+ byte[] data = new byte[1023];
StringBuffer text = new StringBuffer();
- do {
- data = new byte[1023];
- int len;
- while ((len = inStream.read(data)) > 0) {
- byte[] temp = new byte[len];
- System.arraycopy(data, 0, temp, 0, len);
- text.append(Base64.encode(temp));
- }
-
- } while (inStream.available() > 0);
-
+ int len;
+ while ((len = inStream.read(data)) != -1) {
+ text.append(Base64.encode(data, 0, len));
+ }
return new QName(text.toString());
} catch (Exception e) {
throw new OMException(e);
--- snap ---
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org
[jira] Resolved: (WSCOMMONS-142) Inefficient code in TextImpl and
OMTextImpl
Posted by "Davanum Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WSCOMMONS-142?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Davanum Srinivas resolved WSCOMMONS-142.
----------------------------------------
Resolution: Fixed
> Inefficient code in TextImpl and OMTextImpl
> -------------------------------------------
>
> Key: WSCOMMONS-142
> URL: https://issues.apache.org/jira/browse/WSCOMMONS-142
> Project: WS-Commons
> Issue Type: Bug
> Components: AXIOM
> Reporter: Alexander Veit
>
> The methods getText() and getTextAsQName() in org.apache.axiom.om.impl.dom.TextImpl and org.apache.axiom.om.impl.llom.OMTextImpl allocate duplicate buffers and perform needless array copy operations, if input data is read from an InputStream.
> The attached patches should fix the problem. Note that a base64-encoding bug is also fixed for many cases (see WSCOMMONS-101).
> --- snip ---
> Index: D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java
> ===================================================================
> --- D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (revision 485855)
> +++ D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (working copy)
> @@ -312,21 +312,13 @@
> return getTextFromProperPlace();
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - // int x = inStream.available();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + final byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1024];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return text.toString();
> } catch (Exception e) {
> throw new OMException(e);
> @@ -373,21 +365,13 @@
> return new QName(getTextFromProperPlace());
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1024];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return new QName(text.toString());
> } catch (Exception e) {
> throw new OMException(e);
> --- snap ---
> --- snip ---
> Index: D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java
> ===================================================================
> --- D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (revision 487185)
> +++ D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (working copy)
> @@ -228,21 +228,13 @@
> } else {
> try {
> // TODO we have the following code duplicated in getTextAsQName
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + final byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1023];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return text.toString();
> } catch (Exception e) {
> throw new OMException(e);
> @@ -284,21 +276,13 @@
> return new QName(getTextFromProperPlace());
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1023];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return new QName(text.toString());
> } catch (Exception e) {
> throw new OMException(e);
> --- snap ---
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org
[jira] Commented: (WSCOMMONS-142) Inefficient code in TextImpl and
OMTextImpl
Posted by "Alexander Veit (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WSCOMMONS-142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475656 ]
Alexander Veit commented on WSCOMMONS-142:
------------------------------------------
Davanum,
As
---
package org.apache.axiom.om.util;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.SequenceInputStream;
import junit.framework.TestCase;
public class TextHelperTestCase extends TestCase
{
public void test() throws Exception {
assertEquals("YWFh", TextHelper.toString(getInputStream()));
}
private InputStream getInputStream() {
return new SequenceInputStream(
new ByteArrayInputStream("aa".getBytes()),
new ByteArrayInputStream("a".getBytes()));
}
}
---
shows, the correctness of TextHelper's output depends on the kind of InputStream that provides the data. A patch for this was provided at WSCOMMONS-101.
> Inefficient code in TextImpl and OMTextImpl
> -------------------------------------------
>
> Key: WSCOMMONS-142
> URL: https://issues.apache.org/jira/browse/WSCOMMONS-142
> Project: WS-Commons
> Issue Type: Bug
> Components: AXIOM
> Reporter: Alexander Veit
>
> The methods getText() and getTextAsQName() in org.apache.axiom.om.impl.dom.TextImpl and org.apache.axiom.om.impl.llom.OMTextImpl allocate duplicate buffers and perform needless array copy operations, if input data is read from an InputStream.
> The attached patches should fix the problem. Note that a base64-encoding bug is also fixed for many cases (see WSCOMMONS-101).
> --- snip ---
> Index: D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java
> ===================================================================
> --- D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (revision 485855)
> +++ D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (working copy)
> @@ -312,21 +312,13 @@
> return getTextFromProperPlace();
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - // int x = inStream.available();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + final byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1024];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return text.toString();
> } catch (Exception e) {
> throw new OMException(e);
> @@ -373,21 +365,13 @@
> return new QName(getTextFromProperPlace());
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1024];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return new QName(text.toString());
> } catch (Exception e) {
> throw new OMException(e);
> --- snap ---
> --- snip ---
> Index: D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java
> ===================================================================
> --- D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (revision 487185)
> +++ D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (working copy)
> @@ -228,21 +228,13 @@
> } else {
> try {
> // TODO we have the following code duplicated in getTextAsQName
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + final byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1023];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return text.toString();
> } catch (Exception e) {
> throw new OMException(e);
> @@ -284,21 +276,13 @@
> return new QName(getTextFromProperPlace());
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1023];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return new QName(text.toString());
> } catch (Exception e) {
> throw new OMException(e);
> --- snap ---
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org
[jira] Commented: (WSCOMMONS-142) Inefficient code in TextImpl and
OMTextImpl
Posted by "Davanum Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WSCOMMONS-142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475392 ]
Davanum Srinivas commented on WSCOMMONS-142:
--------------------------------------------
Thanks Alex. please see svn revision 511021.
-- dims
> Inefficient code in TextImpl and OMTextImpl
> -------------------------------------------
>
> Key: WSCOMMONS-142
> URL: https://issues.apache.org/jira/browse/WSCOMMONS-142
> Project: WS-Commons
> Issue Type: Bug
> Components: AXIOM
> Reporter: Alexander Veit
>
> The methods getText() and getTextAsQName() in org.apache.axiom.om.impl.dom.TextImpl and org.apache.axiom.om.impl.llom.OMTextImpl allocate duplicate buffers and perform needless array copy operations, if input data is read from an InputStream.
> The attached patches should fix the problem. Note that a base64-encoding bug is also fixed for many cases (see WSCOMMONS-101).
> --- snip ---
> Index: D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java
> ===================================================================
> --- D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (revision 485855)
> +++ D:/prj3/apache/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/TextImpl.java (working copy)
> @@ -312,21 +312,13 @@
> return getTextFromProperPlace();
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - // int x = inStream.available();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + final byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1024];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return text.toString();
> } catch (Exception e) {
> throw new OMException(e);
> @@ -373,21 +365,13 @@
> return new QName(getTextFromProperPlace());
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1024];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return new QName(text.toString());
> } catch (Exception e) {
> throw new OMException(e);
> --- snap ---
> --- snip ---
> Index: D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java
> ===================================================================
> --- D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (revision 487185)
> +++ D:/prj3/apache/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMTextImpl.java (working copy)
> @@ -228,21 +228,13 @@
> } else {
> try {
> // TODO we have the following code duplicated in getTextAsQName
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + final byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1023];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return text.toString();
> } catch (Exception e) {
> throw new OMException(e);
> @@ -284,21 +276,13 @@
> return new QName(getTextFromProperPlace());
> } else {
> try {
> - InputStream inStream;
> - inStream = this.getInputStream();
> - byte[] data;
> + InputStream inStream = this.getInputStream(); // who closes this?
> + byte[] data = new byte[1023];
> StringBuffer text = new StringBuffer();
> - do {
> - data = new byte[1023];
> - int len;
> - while ((len = inStream.read(data)) > 0) {
> - byte[] temp = new byte[len];
> - System.arraycopy(data, 0, temp, 0, len);
> - text.append(Base64.encode(temp));
> - }
> -
> - } while (inStream.available() > 0);
> -
> + int len;
> + while ((len = inStream.read(data)) != -1) {
> + text.append(Base64.encode(data, 0, len));
> + }
> return new QName(text.toString());
> } catch (Exception e) {
> throw new OMException(e);
> --- snap ---
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org