Closed
Bug 994302
Opened 11 years ago
Closed 10 years ago
disable speculative connections for talos
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 996871
People
(Reporter: jmaher, Unassigned, Mentored)
Details
(Whiteboard: [good first bug][lang=python])
Attachments
(1 file)
(deleted),
patch
|
jmaher
:
review-
|
Details | Diff | Splinter Review |
in bug 992611 we found that speculative connections was causing a lot of random failures/leaks, it is a good idea to do this for talos:
https://bugzilla.mozilla.org/show_bug.cgi?id=992611#c14
here is the change:
https://hg.mozilla.org/releases/mozilla-aurora/rev/4fdcd09ca66d
getting started with talos:
https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code
add the pref here:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l237 (note the format will be slightly different).
I don't know a lot about talos, but I'd still like to help out with this.
Reporter | ||
Comment 2•11 years ago
|
||
Patrick, welcome to Mozilla!
I tried to outline the steps to do this above. This should be an easy fix, a single line actually. I am in irc #ateam, as :jmaher- I would love to help you there or if you are not comfortable with IRC, ask some questions here in the bug.
Follow the steps to getting started with Talos and post a patch in the bug. Talos is in Mecurial, so post a patch as a diff:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ
Hey, everything is set up and talos is working fine. But I'm not really sure how to correctly translate
' user_pref("network.http.speculative-parallel-limit", 0); '
over to PrefConfiguration.py, other than that everything has been going pretty smoothly.
Reporter | ||
Comment 4•11 years ago
|
||
glad to hear you got setup on your own! This is a great question and can be confusing.
If you look at the prefs here:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l237
They are in this format:
'browser.bookmarks.max_backups': 0,
we want to convert user_pref("network.http.speculative-parallel-limit", 0); into that format.
The format in the talos definition is almost identical except that it stores all the preferences/values in a data structure and not defining each preference/value pair inside of user_pref().
This comment is intentionally not answering it directly- I am sure you can figure this out.
One thing to do prior to uploading the patch is verify you can run the ts test.
./talos --app `which firefox` -a ts_paint --develop
that will open and close the browser really fast 20 times and then generate some results. Verify it runs the same before and after your change.
Thanks for working on this!
Alright I've updated the code, and I'm going to start to post the patch as diff.
Scratch that, I've had some difficulties getting mercurial to work, if you've got some time free tomorrow could we meet in an irc for a bit to chat about it?
Reporter | ||
Comment 7•11 years ago
|
||
I will be on irc from about 5am pacific time until about 2pm pacific time- find me in #ateam as :jmaher, or post your mercurial issues here in the bug.
I messed around with mercurial a little bit more and I think that this is the correct format for uploading it to bugzilla.
Comment 9•11 years ago
|
||
The patch format looks good. However, you should include the bug number in your commit message. No need to upload a new patch for it, just something to keep in mind going forward :). The link below provides some more info on writing good commit messages:
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment
Comment 10•11 years ago
|
||
Alright thanks for the heads up, I'll keep that in mind.
Reporter | ||
Comment 11•11 years ago
|
||
Comment on attachment 8407962 [details] [diff] [review]
bug994302.patch
Review of attachment 8407962 [details] [diff] [review]:
-----------------------------------------------------------------
you can make the top line of the message:
Bug 994302 - disable speculative connections for talos
::: talos/PerfConfigurator.py
@@ +259,5 @@
> 'hangmonitor.timeout': 0,
> 'network.proxy.http': 'localhost',
> 'network.proxy.http_port': 80,
> 'network.proxy.type': 1,
> + 'network.http.speculative-parallel-limit', 0
at the end of the line you are missing a ,
Attachment #8407962 -
Flags: review-
Reporter | ||
Comment 12•11 years ago
|
||
Patrick, I am just checking in with you to see if you have figured this out yet? Feel free to ask questions here in bugzilla or on irc!
Flags: needinfo?(Patrick.Mepyans)
Comment 13•11 years ago
|
||
Just tapping on the glass here - Patrick, are you still working on this?
Assignee | ||
Updated•10 years ago
|
Mentor: jmaher
Whiteboard: [good first bug][mentor=jmaher][lang=python] → [good first bug][lang=python]
Comment 14•10 years ago
|
||
This ended up being fixed as part of bug 996871, since it was later found to be blocking bug 1026970 and so was needed more urgently.
Updated•10 years ago
|
Flags: needinfo?(Patrick.Mepyans)
You need to log in
before you can comment on or make changes to this bug.
Description
•