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

[Andrew Wnuk]

On 12/20/2011 06:43 PM, Adam Young wrote:
> 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 at 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-charset-charset-decode-encode
>>   * http://www.theserverside.com/discussions/thread.tss?thread_id=61270
>>
>
>
> The article also states that the issue is being delat with in Java 7.

and this is what we need to verify.

> 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,

Performance is not easily achievable and ignoring it is not the best 
practice for developing servers.

>   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.
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20111221/1b5365d4/attachment.htm>