Closed Bug 124037 Opened 20 years ago Closed 20 years ago

Suggestion from NSS to decrease cert manager loading time.

Categories

(Core Graveyard :: Security: UI, enhancement, P2)

1.0 Branch
enhancement

Tracking

(Not tracked)

VERIFIED FIXED
Future

People

(Reporter: ssaux, Assigned: KaiE)

References

Details

Attachments

(1 file, 3 obsolete files)

From bug 121487

BTW, PMS can decrease the startup time of the CERT Dialog by 1/3 by rearranging
it's code a bit. Currently PMS calls PK11_ListCerts to extract all the certs
from the token, then filters these lists for the certs it wants. In the course
of creating the CERT Dialog it calls PK11_ListCerts 3 separate times. If the
code called PK11_ListCerts once, then sorted the certificates it would reduce
it's bringup time by about 1/3 (assuming PK11_ListCerts is a significant part of
the bringup of the dialog).
Priority: -- → P2
Target Milestone: --- → 2.2
nsbeta1
Keywords: nsbeta1
Keywords: nsbeta1nsbeta1-
Severity: normal → enhancement
Summary: Suggestion from NSS to increate cert manager loading time. → Suggestion from NSS to increase cert manager loading time.
Target Milestone: 2.2 → Future
Summary: Suggestion from NSS to increase cert manager loading time. → Suggestion from NSS to decrease cert manager loading time.
Indeed. Calling PK11_ListCerts happens now 4 times during Cert Manager startup.

My testing shows that 87% of the time is spent within PK11_ListCerts.
8% is spent in Mozilla's sorting code.

Bob, you say we call PK11_ListCerts multiple times to extract all the certs. In
fact we call it with a parameter to request only certain types of certificates,
such as PK11CertListUser or PK11CertListCA.

I can see that the time for PK11_ListCerts(PK11CertListCA) is about 6 times it
takes to call PK11_ListCerts(PK11CertListUser).
> I can see that the time for PK11_ListCerts(PK11CertListCA) is about 6 times it
> takes to call PK11_ListCerts(PK11CertListUser).

That happened only once actually, and was an error in measuring. On average, it
is only about 1.5 times.

Blocks: 121916
PK11_ListCerts spends 99% of its time in nssToken_TraverseCertificates.

Within nssToken_TraverseCertificates, more than 80% of the time are spent in the
callback, which is nssPKIObjectCollection_AddInstanceAsObject.

Assignee: ssaux → kaie
Keywords: nsbeta1-nsbeta1
Basically we are iterating over all the certs in the database to find the certs
PSM is requesting. Since PSM sorts out the certs itself anyway, you can call
PK11_ListCerts() once and get all the certs, then sort them into their various
buckets.

Also, the tip of NSS has several PK11_ListCerts() performance enhancements. They
are pretty extensive, so I doubt anyone would want to try to pull them into 7.0,
but you should see quite a gain on startup of the security manager by just
moving to the new calls.
I implemented the optimization to only call PK11_ListCerts once.

By doing that, the required time went down to 48%.

While working on the code, I noticed that the sort code is very inefficient.
Whenever it needs to compare two certificates, it allocates from 2-6 strings,
each requiring a NSS allocation and a conversion to unicode.

I implemented a hash table that is used to cache the strings needed during sorting.

Now we are down to 33% of the originally needed time to open the cert manager.

On my P3 500 Mhz, and a 1.5 MB cert db, without the patch it takes 23 seconds to
open. With the patch, it takes about 7.5 seconds.
Status: NEW → ASSIGNED
Attached patch Patch v1, down to 48% (obsolete) — Splinter Review
Attached patch Patch v2, down to 33% (obsolete) — Splinter Review
Javi, can you please review?
I think you have it down to pretty much the minimum you can get on your NSS code
base. Picking up a new version of NSS should get your time down to 1-2 seconds
on a database that size.

bob
rangan, can you review Kai's patch as Javi is away this week. Please also take a
look to make sure you use the same optimizations in your code.
Thanks,
Stephane.
*** Bug 82212 has been marked as a duplicate of this bug. ***
Comment on attachment 92548 [details] [diff] [review]
Patch v2, down to 33%

seems fine.
Attachment #92548 - Flags: review+
John, could you please have a look whether my pld hashtable cleanup code is
sufficient?
Keywords: nsbeta1nsbeta1+
Attached patch Patch v3 (obsolete) — Splinter Review
I had a chat on IRC with jkeiser. He had a suggestion to avoid some unnecessary
allocations. This does not seem to have optimized the required time, since >90%
of the time is still spent in NSS.

But anyway, its a good idea to avoid unnecessary allocations.
Attachment #92548 - Attachment is obsolete: true
Comment on attachment 92985 [details] [diff] [review]
Patch v3

carrying forward r=rangansen. r=jkeiser on hash table cleanup/init code.
Attachment #92985 - Flags: review+
Jag, can you please review?
Attached patch Patch v4Splinter Review
I found a problem with the previous patch.

I changed the signature of LoadCerts, but some JavaScript code still requires
the function with the old interface.

Attaching a fixed patch that supports both methods.
Attachment #92985 - Attachment is obsolete: true
Comment on attachment 93155 [details] [diff] [review]
Patch v4

r=rangansen
Attachment #93155 - Flags: review+
Comment on attachment 93155 [details] [diff] [review]
Patch v4

sr=jag
Attachment #93155 - Flags: superreview+
Blocks: 157927
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Verified on Win2000 8/8/02 trunk. I have 11 personal certs, and opening the cert 
manager takes about 1-2 seconds compared with the 8/5 build which takes about 
3-4 seconds.
Status: RESOLVED → VERIFIED
Product: PSM → Core
Version: psm2.2 → 1.0 Branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.