83247 - Improve performance of "SEC_ASN1Decode"

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P3)

Description

[zhoubin]

In NSS,Many places call "SEC_ASN1Decode",so many many times are wasted,which 
debases performance greatly!!!This function is a lower layer fucntion,and 
should be run faster.
I think the reason is:The recursion is used when parsing the coding.

Comment 1

[Wan-Teh Chang]

Thank you for the bug report.  Did you profile NSS and find that
it is wasting a lot of time in SEC_ASN1Decode?

Comment 2

[zhoubin]

In my application,I need process many certificates(get dn,subject,public 
key,private key) which cause call SEC_ASN1Decode so many times.And I test it by 
Rational Quantify,and find that SEC_ASN1Decode use a lot of time.

Comment 3

[Robert Relyea]

The SEC_ASN1Decode is slow because it is complicated. The goal was to be able to
parse and ANS1 template thrown at it, while streaming. Nice and general case.
Dog slow with certificates. We get away with this most of the time because the
code tries to parse certificates only once (presuming you aren't calling
DecodeCertificate), and leave the cached parsed certificate in the temp db.

The 3.4 and 4.0 changes should streamline this a bit, including possibly using a
smaller, faster, simplier decoder and leaving the large one for parsing incoming
S/MIME messages.

Comment 4

[Wan-Teh Chang]

Changed the QA contact to Bishakha.

Comment 5

[Wan-Teh Chang]

*** Bug 150089 has been marked as a duplicate of this bug. ***

Comment 6

[Wan-Teh Chang]

Moved Julien's comment from bug 150089:

One finding that came out of that defect is that SEC_ASN1_SKIP doesn't really do
the right thing and still consumes a significant amount of time proportional to
the size of the data block to skip. This is definitely an area where improvement
is possible.

Comment 7

[Julien Pierre]

Please see bug # 149816 , which contains a new, much faster ASN.1 DER decoder.
This is good only for non-streaming DER. BER or streaming isn't supported.

Comment 8

[Julien Pierre]

This is half-fixed - all the DER cases are, that is. See bug 161215 .

The new SEC_ASN1_QuickDERDecodeItem could probably also be made to handle BER
relatively quickly, by making just one copy of the input. The one thing that
couldn't be added to it easily is streaming.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Changing target milesstone to --.  This isn't a stan bug.  It reports a bug 
in a 3.x interface.  

This bug suggests certain problems, but doesn't actually establish any of them
as proven, including:

a) re-decoding certificates repeatedly.  Caching (which I believe was added
after this bug was filed) should reduce this, unless we are redecoding the cert
when we already have a decoded copy sitting there.  All functions that return
parts of a cert should check to see if it is already deocded before decoding it
again.

b) not SKIPing over cert parts efficiently.  SKIP can only be efficient for 
definite length encoding.  I guess certs are usually DER, which inplies definite
length.  

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

I believe NSS is now using the new QuickDERDecoder for all cert processing.
So, can we mark this bug fixed?  

Comment 11

[Julien Pierre]

Yes. It should be noted that for situations where SEC_ASN1DecodeItem still has
to be used, ie. BER decoding, it helps performance quite a bit to use an arena
with a high chunk size. non-arena decoding needs to be avoided due to the leak
potential and low performance, but unfortunately the API supported it
historically, so we can't change that.