review request for 6798511/6860431: Include functionality of Surrogate in Character

[Ulf Zibis]

Hi all,

I have put all important things of sun.nio.cs.Surrogate to java.* 
packages, I guess. We likely could get rid of it.

See: https://bugs.openjdk.java.net/show_bug.cgi?id=100104


Some comments:
================
     public static int toChars(int codePoint, char[] dst, int dstIndex) {
-        if (codePoint < 0 || codePoint > MAX_CODE_POINT) {
-            throw new IllegalArgumentException();
-        }
-        if (codePoint < MIN_SUPPLEMENTARY_CODE_POINT) {
-            dst[dstIndex] = (char) codePoint;
-            return 1;
-        }
-        toSurrogates(codePoint, dst, dstIndex);
+        if (codePoint >= MIN_CODE_POINT) {
+            if (codePoint < MIN_SUPPLEMENTARY_CODE_POINT) {
+                dst[dstIndex] = (char)codePoint;
+                return 1;
+            }
+            if (codePoint <= MAX_CODE_POINT)
+                return toSurrogates(codePoint, dst, dstIndex);
+        }
+        throw new IllegalArgumentException();
+    }
----------------
saves 1 compare in case of code point is in BMP
================
+    public static char[] toChars(int codePoint) {
+        if (codePoint >= MIN_CODE_POINT) {
+            char[] result;
+            if (codePoint < MIN_SUPPLEMENTARY_CODE_POINT) {
+                result = C1.clone();
----------------
I guess cloning is faster than new instantiation, as fresh one has to be 
initialized by 0s, but not sure
================
+    static int toSurrogates(int codePoint, char[] dst, int index) {
+        // We write elements "backwards" to guarantee all-or-nothing // 
TODO: isn't forward faster ?
+        dst[index+1] = lowSurrogate(codePoint);
+        dst[index] = highSurrogate(codePoint);
----------------
IMO this guarantee is nowhere needed, so why potentially waste performance ?
================
-        // Pass 2: Allocate and fill in char[]
-        char[] v = new char[n];
-        for (int i = offset, j = 0; i < offset + count; i++) {
-            int c = codePoints[i];
-            if (c < Character.MIN_SUPPLEMENTARY_CODE_POINT) {
-                v[j++] = (char) c;
-            } else {
-                Character.toSurrogates(c, v, j);
-                j += 2;
-            }
-        }
-
-        this.value  = v;
+        // Pass 2: Allocate and fill in char[]
+        for (int i = offset, j = 0; i < offset + count; i++) {
+            int c = codePoints[i];
+            if (c < Character.MIN_SUPPLEMENTARY_CODE_POINT)
+                value[j++] = (char)c;
+            else
+                j += Character.toSurrogates(c, value, j);
         }
----------------
I guess HotSpot is aware of immutability of String, so no need to 
locally buffer v expletive to save indirection and against concurrent 
modifications.
================


IMO code nesting should indent 4 spaces, but line continuation should 
indent 8 spaces

-Ulf