Re: [Pki-devel] [PATCH] 4 Replaced byte-to-char & char-to-byte converters.

If the only questions are those of performance, than it is a non issue for now. On 12/20/2011 09:19 PM, Matthew Harmsen wrote: > On 12/16/11 17:24, Endi Sukma Dewata wrote: >> On 12/13/2011 4:25 PM, Adam Young wrote: >>> On 12/13/2011 02:03 PM, Andrew Wnuk wrote: >>>> Hopefully this patch was extremely well tested since it touches ASN.1 >>>> encoding. >>> Do you have a specific set of tests that we should be running? >> >> I just posted patch #5 which contains some unit tests for the >> converters. I've also updated and split patch #4 into #4-2a >> (renaming the files) and #4-2b (replacing the implementation). >> >> Patch #5 should be applied before #4-2a/b to make sure there's no >> regression. >> >> Mozilla JSS library also has some ASN.1 code, should we use them to >> implement the converters? >> >> http://mxr.mozilla.org/mozilla/source/security/jss/org/mozilla/jss/asn1/ >> >> >> >> >> _______________________________________________ >> Pki-devel mailing list >> Pki-devel(a)redhat.com >> https://www.redhat.com/mailman/listinfo/pki-devel > Endi, > > Andrew and I have been looking at these changes, and we have the > following concerns: > > (1) First of all, we looked up the bug where some of the sun.io.* > classes were deprecated (which mentions java.nio.charset and sun.nio.cs): > > * http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4948149 > > (2) There appears to be considerable concern on performance > degradation when using "Charset": > > * http://stackoverflow.com/questions/2098137/fast-alternative-to-java-nio-c... > * http://www.theserverside.com/discussions/thread.tss?thread_id=61270 > The article also states that the issue is being delat with in Java 7.

We are standardizing on Java 7 for this release, so it should not be an issue. > (3) Additionally, there have been alterations in the CS code in the > past to fix problems encountered with the sun.io.* classes: > > * pki/base/util/src/netscape/security/util/ByteToCharPrintable.java > > Although we have not gotten to the unit tests, we suspect that these > will be great to have regardless of the direction that is decided > upon. However, due to our concerns regarding performance, could we > have some tests added (unit or a test tool) which obtain performance > results for the existing methods versus the proposed newer methods? > > If no discernable performance issues are encountered, most of these > changes appear to be acceptable -- the questionable one being the > replacing of the code that addressed issue (3) above. > > If there is an unacceptable degradation in performance, perhaps we > could utilize some customized Java classes to perform these > functions. As an alternative approach, as Endi alluded to, there is > some ASN.1 code in Mozilla JSS, and speaking with Bob Relyea, we have > been postulating on how much work would be involved to write JNI > bindings via JSS to the ASN.1 encoders/decoders contained in NSS > instead of moving this code to use java.nio.* classes. > Theoretically, we may limit our performance issues in exchange for > the extra work involved in making the effort to standardize on the > NSS ASN.1 engine (although I don't know if this will resolve issues > such as (3) noted above). > > -- Matt I am not worried about performance at this point,

it is way too premature. I am only concerned with correctness. The deprecated classes are going away, and we need an alternative to them. We will have a deliberate performance and tuning iteration once the code base has been restructured. If we find at that stage that there are still performance issues, we can look into writing a custom tuned solution, but based on overall system performance.