Hi David, > > Ingo has done some work to clean up the encoding/decoding in > > InputStreamReader and OutputStreamWriter. Basically it uses NIO buffers > > now at both ends of the encoder or decoder which makes the code more > > clean and hopefully more efficient. > > > > It also improves the read() and write() methods to avoid allocating > > small temporary arrays on each read/write operation. > > > > This still passes all Mauve tests for these classes (ok, not so many > > tests there..) > > > > Any comments? If nobody complains I'll check this in tomorrow. > > > > I don't see how you can claim that it is more efficient. Do you have > any numbers to back that claim up?
No numbers yet. Ingo has done this work and at least it wasn't slower. I'll ask him for details. This is for the JamaicaVM though. JamaicaVM has relatively slow array access but the ByteBuffers used here are backed by arrays too, so this would make no big difference. > On most free JVMs that I know of (libgcj), array accesses are much > faster than the corresponding actions on a java.nio.Buffer. I don't think so. The normal bytebuffer is also only an array. The accessor methods should not make much difference. > In theory > the java.nio.Buffer accesses could be optimized as described here: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27982 but I don't think > anyone is doing this yet. It is not really the issue of arrays vs. ByteBuffers. The way how we did the byte[]->ByteBuffer->CharBuffer->char[] was ugly at least and inefficient at worse (as I said, no numbers yet but that's my feeling). > We have working code. Cleanup is nice, but I don't want to see it done > at the expense of making the runtime slower. Sure. > My gut feeling is that this patch would make the Reader and Writer > operations slower. If you can show that on libgcj that there is no > difference on a benchmark that moves a lot of data through the changed > code, then I withdraw my objection. I'll make some benchmarks asap (With JamVM and Cacao though, as this is what I have in my setup). /Roman
