Closed Bug 1361601 Opened 2 years ago Closed 2 years ago

Remove system-info.getProperty("host")

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mcmanus, Unassigned)

References

Details

Attachments

(1 file)

PR_GetSystemInfo(PR_SI_HOSTNAME, ..) turns out to be quite expensive on windows because it silently initializes winsock.

nsSystemInfo::Init() does this to add the hostname to the propertyBag under the "host" key.

There doesn't seem to be anything in the codebase that accesses that property - so we should remove it. It also seems like a per user tracker waiting to accidentally happen somewhere.

This is a try run that indicates nothing accesses getProperty("host").. nothing came up in an addon search either though its a hard thing to search for.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=394e8b31c3490fd118dd8f3ef12033975c0809a4

https://bugzilla.mozilla.org/show_bug.cgi?id=1361087 shows the original performance problem.
Comment on attachment 8863961 [details]
Bug 1361601 - Remove nsSystemInfo.getProperty("host")

https://reviewboard.mozilla.org/r/135690/#review138820
Attachment #8863961 - Flags: review?(nfroyd) → review+
Pushed by mcmanus@ducksong.com:
https://hg.mozilla.org/integration/autoland/rev/040f581bbbd9
Remove nsSystemInfo.getProperty("host") r=froydnj
I remember there was a bug some time a go, where we were stack on main thread doing dns. 
As I remember we needed that for something. I am not sure if this was the place that was making problems and removing this will cause other problems.

I think bsmedberg was commenting in that bug. I will need info him: Do I remember correctly, can you check if it is fine to remove this (if  you know)? Thanks.
Flags: needinfo?(benjamin)
there was (is?) a sync dns  resolution interface that we had some plugin users of that caused fits.. maybe that's what you're thinking of? It wouldn't be relevant here if that's it..
(In reply to Patrick McManus [:mcmanus] from comment #6)
> there was (is?) a sync dns  resolution interface that we had some plugin
> users of that caused fits.. maybe that's what you're thinking of? It
> wouldn't be relevant here if that's it..

I think it was something else, it had to do with a profile, but maybe I am wrong.
https://hg.mozilla.org/mozilla-central/rev/040f581bbbd9
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
ISTR a bug, but I'm pretty certain it wasn't this API.
Flags: needinfo?(benjamin)
Depends on: 1369285
(In reply to Patrick McManus [:mcmanus] from comment #0)
> There doesn't seem to be anything in the codebase that accesses that
> property

Sync does (bug 1369285) and http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/devtools/shared/discovery/discovery.js#179 also does. It looks like there should be a bug on file for that?
Flags: needinfo?(mcmanus)
probably a question for devtools
Flags: needinfo?(mcmanus)
Depends on: 1370148
You need to log in before you can comment on or make changes to this bug.