EKU test fixtures add too much time to builds

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mt, Assigned: keeler)

Tracking

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 affected, firefox47 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
I just looked into the 1 minute of extra time that was added to the build in Bug 1171819.
 
Basically, every line in security/manager/ssl/tests/unit/test_cert_eku/moz.build identifies a new certificate that is signed.  This takes a huge amount of time and every mozilla developer pays that price.  A little bit of optimization would go a long way here.

If the default certificate were changed to an EC cert, that would speed this up by a huge margin.  As it is - at least as far as I can tell, ALL of these are signed with a 2048-bit RSA key, which is a significant undertaking.  (I couldn't see any use of the "signature:" directive in the certspec files in this directory.  

A good implementation of P-256 should run around 3 times faster, which is a non-trivial gain.

Of course, these files should probably be generated on demand.  If I were to guess, I'd say that these are now generated many more times than they are used in the aggregate.
(In reply to Martin Thomson [:mt:] from comment #0)
> If the default certificate were changed to an EC cert, that would speed this
> up by a huge margin.  As it is - at least as far as I can tell, ALL of these
> are signed with a 2048-bit RSA key, which is a significant undertaking.  (I
> couldn't see any use of the "signature:" directive in the certspec files in
> this directory.  
> 
> A good implementation of P-256 should run around 3 times faster, which is a
> non-trivial gain.

I gave this a go and in practice it looks like it saves about 10 seconds out of 55, which is not insignificant, but unfortunately it's nowhere near 3x faster. Maybe the library is inefficient.

> Of course, these files should probably be generated on demand.  If I were to
> guess, I'd say that these are now generated many more times than they are
> used in the aggregate.

Good call. I bet we could hook into the test harness and have it generate the files.
As of bug 1248099 landing, the xpcshell extended key usage tests are mostly redundant. I'm going to remove all but the few that we actually care about as integration tests. This should remove ~500 certificates, which accounts for about half of the build-time generated certificates. If this doesn't bring build times back into a reasonable range, we can look into removing (or merging) redundant certificates.
Depends on: 1248099
Assignee: nobody → dkeeler
Comment on attachment 8723286 [details]
MozReview Request: bug 1199850 - remove unnecessary PSM xpcshell extended key usage tests r?Cykesiopka,jcj

https://reviewboard.mozilla.org/r/36433/#review33029

I don't see anything peculiar in this deduplication commit. LGTM.
Attachment #8723286 - Flags: review?(jjones) → review+

Comment 5

3 years ago
Comment on attachment 8723286 [details]
MozReview Request: bug 1199850 - remove unnecessary PSM xpcshell extended key usage tests r?Cykesiopka,jcj

https://reviewboard.mozilla.org/r/36433/#review33303

Nice! r+ with the minor change noted below.

::: security/manager/ssl/tests/unit/test_cert_eku/moz.build:1
(Diff revision 1)
>  # AUTOGENERATED FILE, DO NOT EDIT

This line should be removed now.
Attachment #8723286 - Flags: review?(cykesiopka.bmo) → review+

Updated

3 years ago
Blocks: 1248217
Comment on attachment 8723286 [details]
MozReview Request: bug 1199850 - remove unnecessary PSM xpcshell extended key usage tests r?Cykesiopka,jcj

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36433/diff/1-2/

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/29095b39290b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.