Joel Maher ( :jmaher ) (UTC -8) Reporter
	Description • 10 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).

	Patrick
	Comment 1 • 10 years ago

I don't know a lot about talos, but I'd still like to help out with this.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 2 • 10 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

	Patrick
	Comment 3 • 10 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.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 4 • 10 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!

	Patrick
	Comment 5 • 10 years ago

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

	Patrick
	Comment 6 • 10 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?

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 7 • 10 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.

	Patrick
	Comment 8 • 10 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.

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 10 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

	Patrick
	Comment 10 • 10 years ago

Alright thanks for the heads up, I'll keep that in mind.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 11 • 10 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 ,

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 12 • 10 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!

	Mike Hoye [:mhoye]
	Comment 13 • 10 years ago

Just tapping on the glass here - Patrick, are you still working on this?

	Ed Morley [:emorley]
	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.