Search service should do its own countryCode fetching as-needed instead of having system-info init get it eagerly
Categories
(Firefox :: Search, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: Gijs, Assigned: mcheang)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fxperf:p2] [fxperfsize:S])
Attachments
(1 file)
Bug 1553558 - collect countryCode in system info off the main thread. r?mconley,standard8,daleharvey
47 bytes,
text/x-phabricator-request
|
Details | Review |
Talking to the registry for data that only the search service needs and only for some telemetry is not really the best for performance, considering when the system info service is initialized.
We should check that the telemetry is still needed. If not, we can just remove all the associated code. If it is needed, we can move the OS-based lookups elsewhere, e.g. to a property on xpcom/system/nsIXULRuntime.idl that does the work the first time someone asks for it, and then do the requesting only after startup, off an idle task.
Drew, do you know if this telemetry (added in bug 1121340) is still important?
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 2•5 years ago
•
|
||
The telemetry is still important, we need to keep it around a while longer.
Comment 3•5 years ago
|
||
Perhaps we can make the country code something that Telemetry is responsible for getting (lazily, and maybe off of the main thread).
Hey, Georg - is there precedent for Telemetry gathering and hosting information, not to send as part of a ping, but for other parts of Firefox to make decisions on? Specifically, would it be outlandish to have Telemetry be responsible for retrieving and storing the platform OS's country code value for the Search Service to use here?: https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/toolkit/components/search/SearchService.jsm#186-212
rather than storing the country code in nsSystemInfo (which isn't currently designed to get any of its values lazily / off-main-thread).
Comment 4•5 years ago
|
||
(In reply to Mike Conley (:mconley) (:⚙️) from comment #3)
Hey, Georg - is there precedent for Telemetry gathering and hosting information, not to send as part of a ping, but for other parts of Firefox to make decisions on? Specifically, would it be outlandish to have Telemetry be responsible for retrieving and storing the platform OS's country code value for the Search Service to use here?: https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/toolkit/components/search/SearchService.jsm#186-212
There is no precedent for that AFAIK. I think currently the Telemetry interface are not ideal for exposing information to code inside Firefox.
Is there value from exposing this through Telemetry vs. another interface?
ni?chutten for his perspective.
Comment 5•5 years ago
|
||
Telemetry at present is write-only except for test APIs, so it doesn't seem like it'd be a good fit.
Reporter | ||
Comment 6•5 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #2)
The telemetry is still important, we need to keep it around a while longer.
Can you elaborate a bit? We've had this telemetry for 3+ years. Do we still not know whether our geoip determination is more/less accurate than the OS one? Does that change a lot? Have we ever switched away from our geoip one to the OS one?
FWIW, analogous to the patch in bug 1553540 we can probably remove this from the existing Init() in the system info service, and instead add a promise attribute to the nsISystemInfo idl that fetches the mac/windows bits OMT, and have the search service code call that when it actually wants this stuff.
Comment 7•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #6)
(In reply to Mike de Boer [:mikedeboer] from comment #2)
The telemetry is still important, we need to keep it around a while longer.
Can you elaborate a bit? We've had this telemetry for 3+ years. Do we still not know whether our geoip determination is more/less accurate than the OS one? Does that change a lot? Have we ever switched away from our geoip one to the OS one?
Well, it's been a while indeed and we've only now solidified the plans to implement changes here in Q3 of this year with the goal to improve the accuracy. This probe will tell us if we succeeded, eventually.
Updated•5 years ago
|
Assignee | ||
Comment 8•5 years ago
|
||
Comment 9•5 years ago
|
||
ni?ing tcampbell for a quick second opinion on https://phabricator.services.mozilla.com/D39343#inline-240889.
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
bugherder |
Description
•