Re: [Pki-devel] [PATCH] 4 Replaced byte-to-char & char-to-byte converters.
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