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

[Andrew Wnuk]

On 12/21/2011 11:23 AM, Ade Lee wrote:
>
>>> 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).
>>
>> I checked the code, it looks like some of Mozilla's JSS code also relies
>> on the Charset API. JNI calls have some overhead too, so we need to know
>> how it's going to be used to avoid negative performance impact.
>>
>
> I agree the performance is a concern, and is something that we should
> keep in mind, but it is not clear that differences in performance in
> these classes will degrade the performance of the system.  The only way
> to determine where the system is really spending its time is by running
> it through a profiler and analyzing the results.  We may find that the
> server spends comparatively no time in this code.

I think "may" is a proper keyword here.  If someone, calls "Charset API" 
a "bottleneck" that should be good enough warning to do some simple 
testing to verify it.

>
>
> On the other hand, we have a compelling reason to change these classes.
> The classes are deprecated - and will go away - and then everything will
> stop working.
>
> I suggest that we review this patch for correctness.

I think Matt's email was clear. This patch is fine but we had 3 concerns 
which were listed in his email.

>    Is the encoding
> correct?

There is no simple test to prove this or to replace years of 
compatibility testing (case like (3) can be good example of this).

>    Do the unit tests cover enough - or have the right inputs to
> validate the correctness of the conversion?  It seems like there were
> situations like (3) where perhaps more testing is required.

Agreed, there was always apparent reason for fixes like (3).

>
>
> And we should schedule a performance testing and profiling task for next
> year.  We will always have the 8.x code as a benchmark.  If at that
> time, we find that these classes cause performance problems, we can
> always look into custom classes or maybe JSS code.
>
> It seems to me that talking about custom classes before having any
> profiling data is putting the cart before the horse.
>
> Ade
>
>