disable speculative connections for talos

RESOLVED DUPLICATE of bug 996871

Status

defect
RESOLVED DUPLICATE of bug 996871
5 years ago
5 years ago

People

(Reporter: jmaher, Unassigned, Mentored)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=python])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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).

Comment 1

5 years ago
I don't know a lot about talos, but I'd still like to help out with this.
(Reporter)

Comment 2

5 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

Comment 3

5 years ago
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

5 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!
Blocks: 617414

Comment 5

5 years ago
Alright I've updated the code, and I'm going to start to post the patch as diff.

Comment 6

5 years ago
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

5 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.

Comment 8

5 years ago
I messed around with mercurial a little bit more and I think that this is the correct format for uploading it to bugzilla.
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

5 years ago
Alright thanks for the heads up, I'll keep that in mind.
(Reporter)

Comment 11

5 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

5 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)
Just tapping on the glass here - Patrick, are you still working on this?
Blocks: 1023483
Mentor: jmaher
Whiteboard: [good first bug][mentor=jmaher][lang=python] → [good first bug][lang=python]
Blocks: 1026970
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.
No longer blocks: 617414, 1023483, 1026970
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 996871
Flags: needinfo?(Patrick.Mepyans)
You need to log in before you can comment on or make changes to this bug.