Closed
Bug 617111
Opened 14 years ago
Closed 14 years ago
Avoid initializing NSS at all costs (ts regression)
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla2.0b9
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: sdwilsh, Assigned: sdwilsh)
References
Details
(Whiteboard: [fixed-in-places])
Attachments
(1 file, 1 obsolete file)
5.40 KB,
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
In bug 607117, we ended up initializing NSS during startup. This is the cause of our current Ts regression, so we need to stop doing that. Shaver suggested I steal jsmath.cpp's random number implementation if it works. It looks to be a suitable PRNG implementation (https://mxr.mozilla.org/mozilla-central/source/js/src/jsmath.cpp#514), and this patch implements it for Places code. If NSS initialization ever gets small enough, we can switch to using it again, but for now this will have to do.
Attachment #495598 -
Flags: review?(dtownsend)
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → betaN+
Assignee | ||
Comment 1•14 years ago
|
||
Try server data (just running xpcshell since that will give us the coverage we want): http://tbpl.mozilla.org/?tree=MozillaTry&rev=6b6ba188da29
Assignee | ||
Comment 2•14 years ago
|
||
Comment on attachment 495598 [details] [diff] [review] v1.0 Looking closer at the output of our test for this, it looks like the last character in our guid is always K, which is not desireable. Going to improve the test to catch this problem, and figure out what is up :(
Attachment #495598 -
Attachment is obsolete: true
Attachment #495598 -
Flags: review?(dtownsend)
Assignee | ||
Comment 3•14 years ago
|
||
Better approach. Making sure this works on unix via the try server first though...
Assignee | ||
Comment 4•14 years ago
|
||
Comment on attachment 495679 [details] [diff] [review] v2.0 Try server appears to be OK with this.
Attachment #495679 -
Flags: review?(dietrich)
Comment 5•14 years ago
|
||
Is this confirmed on Places to resolve the regression?
Comment 6•14 years ago
|
||
Comment on attachment 495679 [details] [diff] [review] v2.0 r=me
Attachment #495679 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #5) > Is this confirmed on Places to resolve the regression? Past expiriemnts showed that it was, but after yesterday's merge, and early talos results with this patch, it didn't look like it. I'll have more data later today.
Assignee | ||
Comment 8•14 years ago
|
||
Try server numbers show a small regression there still, which is annoying. However this did make a bunch of the regression go away. http://hg.mozilla.org/projects/places/rev/f0110c8264c8
Whiteboard: [fixed-in-places]
Comment 9•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/f0110c8264c8
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b9
You need to log in
before you can comment on or make changes to this bug.
Description
•