Closed
Bug 124037
Opened 23 years ago
Closed 22 years ago
Suggestion from NSS to decrease cert manager loading time.
Categories
(Core Graveyard :: Security: UI, enhancement, P2)
Tracking
(Not tracked)
VERIFIED
FIXED
Future
People
(Reporter: ssaux, Assigned: KaiE)
References
Details
Attachments
(1 file, 3 obsolete files)
26.95 KB,
patch
|
rangansen
:
review+
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
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).
Reporter | ||
Updated•23 years ago
|
Priority: -- → P2
Target Milestone: --- → 2.2
Reporter | ||
Updated•23 years ago
|
Assignee | ||
Updated•23 years ago
|
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
Assignee | ||
Updated•23 years ago
|
Summary: Suggestion from NSS to increase cert manager loading time. → Suggestion from NSS to decrease cert manager loading time.
Assignee | ||
Comment 2•23 years ago
|
||
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).
Assignee | ||
Comment 3•23 years ago
|
||
> 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.
Assignee | ||
Comment 4•23 years ago
|
||
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.
Comment 5•23 years ago
|
||
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.
Assignee | ||
Comment 6•23 years ago
|
||
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
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 years ago
|
||
Assignee | ||
Comment 9•23 years ago
|
||
Javi, can you please review?
Comment 10•23 years ago
|
||
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
Reporter | ||
Comment 11•23 years ago
|
||
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.
Assignee | ||
Comment 12•23 years ago
|
||
*** Bug 82212 has been marked as a duplicate of this bug. ***
Comment 13•23 years ago
|
||
Comment on attachment 92548 [details] [diff] [review] Patch v2, down to 33% seems fine.
Attachment #92548 -
Flags: review+
Assignee | ||
Comment 14•23 years ago
|
||
John, could you please have a look whether my pld hashtable cleanup code is sufficient?
Assignee | ||
Updated•23 years ago
|
Assignee | ||
Comment 15•23 years ago
|
||
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
Assignee | ||
Comment 16•23 years ago
|
||
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+
Assignee | ||
Comment 17•23 years ago
|
||
Jag, can you please review?
Assignee | ||
Comment 18•23 years ago
|
||
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 19•22 years ago
|
||
Comment on attachment 93155 [details] [diff] [review] Patch v4 r=rangansen
Attachment #93155 -
Flags: review+
Comment 20•22 years ago
|
||
Comment on attachment 93155 [details] [diff] [review] Patch v4 sr=jag
Attachment #93155 -
Flags: superreview+
Assignee | ||
Comment 21•22 years ago
|
||
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 22•22 years ago
|
||
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
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•