JDK-6233838 : improving charset implementation maintainability and performance
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.nio.charsets
  • Affected Version: 6,6u18
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: generic,linux_redhat_5.0
  • CPU: generic,x86
  • Submitted: 2005-02-28
  • Updated: 2011-05-17
  • Resolved: 2011-05-17
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7
6u21-revFixed 7 b06Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Description
While reviewing the fix for 

6233303 nio ISO2022JP.Encoder is broken unmappableCharacterAction is 

I noticed a number of cases of dead code, duplicated code, and unnecessary
code calls in ISO2022JP.Encoder.encodeLoop.

This code would be much more maintainable and better performing if
these kinds of changes were applied to all NIO charsets.

There is no need to check for surrogateness, as far as I can see,
except possibly for better error diagnosis.  And certainly there
is no need to check for it unless the char is unmappable, i.e.
encodeDouble fails.

outputSize appears to be unused.

The code for backslash and tilde can be unified.

(These changes are completely untested)

Similar changes can be applied to other encodeLoop methods and
other encodings.

--- /tmp/geta24581	2005-02-28 09:18:30.337098600 -0800
+++ ISO2022_JP.java	2005-02-26 17:17:57.951819000 -0800
@@ -383,131 +383,89 @@
 	private CoderResult encodeArrayLoop(CharBuffer src,
 					    ByteBuffer dst)
 	{
 	    char[] sa = src.array();
 	    int sp = src.arrayOffset() + src.position();
 	    int sl = src.arrayOffset() + src.limit();
 	    assert (sp <= sl);
-	    sp = (sp <= sl ? sp : sl);
+
 	    byte[] da = dst.array();
 	    int dp = dst.arrayOffset() + dst.position();
 	    int dl = dst.arrayOffset() + dst.limit();
 	    assert (dp <= dl);
-	    dp = (dp <= dl ? dp : dl);
 
-	    int outputSize = 0;
+	    int index;
 
 	    try {
 		while (sp < sl) {
-		    int newMode = currentMode;
 		    char c = sa[sp];
 
                     if (c <= '\u007F') {
 			if (currentMode != ASCII) {
-			    if (dl - dp < 4)
+			    if (dl - dp < 3 + 1)
 				return CoderResult.OVERFLOW;
 			    da[dp++] = (byte)0x1b;
 			    da[dp++] = (byte)0x28;
 			    da[dp++] = (byte)0x42;
-			    da[dp++] = (byte)c;
-			    newMode = ASCII;
-			    sp++;
+			    currentMode = ASCII;
 			}
-			else {
-			    if (dl - dp < 1)
-				return CoderResult.OVERFLOW;
-			    da[dp++] = (byte)c;
-			    sp++;
-		        }
+			if (dl - dp < 1)
+			    return CoderResult.OVERFLOW;
+			da[dp++] = (byte)c;
+			sp++;
 		    }
 		    // Is it a single byte kana?
 		    else if (c >= 0xff61 && c <= 0xff9f) {
 			if (currentMode != JISX0201_1976_KANA) {
-			    if (dl - dp < 4)
+			    if (dl - dp < 3 + 1)
 				return CoderResult.OVERFLOW;
 			    da[dp++] = (byte)0x1b;
 			    da[dp++] = (byte)0x28;
 			    da[dp++] = (byte)0x49;
-			    da[dp++] = (byte)(c - 0xff40);
-			    newMode = JISX0201_1976_KANA;
-			    sp++;
-			} else {
-			    if (dl - dp < 1)
+			    currentMode = JISX0201_1976_KANA;
+			} 
+			if (dl - dp < 1)
+			    return CoderResult.OVERFLOW;
+			da[dp++] = (byte)(c - 0xff40);
+			sp++;
+		    }
+		    else if ((index = encodeDouble(c)) != 0) {
+			if (currentMode != JISX0208_1983) {
+			    if (dl - dp < 3 + 2)
 				return CoderResult.OVERFLOW;
-			    da[dp++] = (byte)(c - 0xff40);
-			    sp++;
+			    da[dp++] = (byte)0x1b;
+			    da[dp++] = (byte)0x24;
+			    da[dp++] = (byte)0x42;
+			    currentMode = JISX0208_1983;
 			}
+			if (dl - dp < 2)
+			    return CoderResult.OVERFLOW;
+			da[dp++] = (byte)(index >> 8);
+			da[dp++] = (byte)(index & 0xff);
+			sp++;
 		    }
-		    else if (c == '\u00A5') {
+		    else if (c == '\u00A5' /* backslash */ ||
+			     c == '\u203E' /* tilde */) {
 			if (currentMode != JISX0201_1976) {
-			    if (dl - dp < 4)
+			    if (dl - dp < 3 + 1)
 				return CoderResult.OVERFLOW;
 			    da[dp++] = (byte)0x1b;
 			    da[dp++] = (byte)0x28;
 			    da[dp++] = (byte)0x4a;
-			    da[dp++] = (byte)0x5c;
-			    newMode = JISX0201_1976;
-			    sp++;
-			} else {
-			    da[dp++] = (byte)0x5C;
-			    sp++;
+			    currentMode = JISX0201_1976;
 			}
-		    }
-		    else if (c == '\u203E') { // is it a tilde?
-			    if (currentMode != JISX0201_1976) {
-			    if (dl - dp < 4)
-				return CoderResult.OVERFLOW;
-				da[dp++] = (byte)0x1b;
-				da[dp++] = (byte)0x28;
-				da[dp++] = (byte)0x4a;
-				da[dp++] = (byte)0x7e;
-				newMode = JISX0201_1976;
-				sp++;
-			    } else {
-				if (dl - dp < 1)
-				    return CoderResult.OVERFLOW;
-				da[dp++] = (byte)0x7e;
-				sp++;
-			    }
-		    }
-		    else {
-			if (dl - dp < 5)
+			if (dl - dp < 1)
 			    return CoderResult.OVERFLOW;
-			int index = encodeDouble(c);
-			if (index != 0) {
-			    if (currentMode != JISX0208_1983) {
-				da[dp++] = (byte)0x1b;
-				da[dp++] = (byte)0x24;
-				da[dp++] = (byte)0x42;
-				da[dp++] = (byte)(index >> 8);
-				da[dp++] = (byte)(index & 0xff);
-				newMode = JISX0208_1983;
-				sp++;
-			    } else {
-				da[dp++] = (byte)(index >> 8);
-				da[dp++] = (byte)(index & 0xff);
-				sp++;
-			    }
-			}
-			else { 
-			    return CoderResult.unmappableForLength(1);
-			}
+			da[dp++] = (byte) (c == '\u00A5' ? 0x5c : 0x7e);
+			sp++;
 		    }
-
-		    if (Surrogate.is(c)) {
-			if (sgp.parse(c, sa, sp, sl) < 0)
-			    return sgp.error();
-			return sgp.unmappableResult();
+		    else { 
+			return CoderResult.unmappableForLength(1);
 		    }
-
-		    if (dl - dp < outputSize)
-			return CoderResult.OVERFLOW;
-		    currentMode = newMode; 
-
-	      }
+		}
 		return CoderResult.UNDERFLOW;
 	    } finally {
 		src.position(sp - src.arrayOffset());
 		dst.position(dp - dst.arrayOffset());
 	    }
 	}
 

###@###.### 2005-2-28 17:23:47 GMT

Comments
EVALUATION yes.
06-01-2007