Closed Bug 994564 Opened 7 years ago Closed 6 years ago

[Wifi] Use different thread for executing wifi command and netutil command.

Categories

(Firefox OS Graveyard :: Wifi, defect, P2)

All
Gonk (Firefox OS)
defect

Tracking

(tracking-b2g:backlog, b2g-v1.3T affected)

RESOLVED INVALID
tracking-b2g backlog
Tracking Status
b2g-v1.3T --- affected

People

(Reporter: chucklee, Assigned: chucklee)

References

Details

(Keywords: perf, Whiteboard: [c=progress p= s= u=])

Attachments

(2 files)

Sometimes it takes a very long to to enabling/disabling wifi.
After some digging, it seems to be that the netutil commands(do_dhcp_request, dhcp_stop, ifc_reset_connections, etc) takes too much time and blocks the upcoming wifi command, because netutil[1] and wifi[2] share the same command thread. [3]
And netutil commands are mostly blocking calls, take do_dhcp_request [4] for example, the function call is blocked until dhcp request is done. If there's wifi command need to be executed, it has to wait until the dhcp request is done.

Since wifi command is for MAC layer and netutil command is for IP layer, these two command should be able to be run in parallel. So I think we can add anther thread for netutil commands and dispatch different command to different thread based on the command.

[1] http://hg.mozilla.org/mozilla-central/file/d68c74e48075/dom/wifi/WifiWorker.js#l161
[2] http://hg.mozilla.org/mozilla-central/file/d68c74e48075/dom/wifi/WifiWorker.js#l162
[3] http://hg.mozilla.org/mozilla-central/file/d68c74e48075/dom/wifi/WifiProxyService.cpp#l254
[4] http://hg.mozilla.org/mozilla-central/file/d68c74e48075/dom/wifi/WifiUtils.cpp#l364
Comment on attachment 8405212 [details] [diff] [review]
WIP - Add thread for netutil command.

Review of attachment 8405212 [details] [diff] [review]:
-----------------------------------------------------------------

Based on our test result yesterday, the patch doesn't seem to improve the wifi startup speed in tarako. 
So maybe we should use the about memory tool to see the memory impact first.

::: dom/wifi/WifiProxyService.cpp
@@ +272,5 @@
> +      commandOptions.mCmd.EqualsLiteral("connect_to_supplicant")) {
> +    mWifiControlThread->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
> +  } else {
> +    mNetUtilControlTrhead->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
> +  }

Can we put the commands to two seperate arrays to make it clearly ?
Attachment #8405212 - Flags: feedback?(vchang)
blocking-b2g: --- → 1.3T?
Keywords: perf
This might not necessarily be a blocker, though if it lands I would like it consider for taking (ie backlog)
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [c=progress p= s= u=]
blocking-b2g: 1.3T? → backlog
Comment on attachment 8405212 [details] [diff] [review]
WIP - Add thread for netutil command.

Review of attachment 8405212 [details] [diff] [review]:
-----------------------------------------------------------------

This seems reasonable to me.

::: dom/wifi/WifiProxyService.cpp
@@ +272,5 @@
> +      commandOptions.mCmd.EqualsLiteral("connect_to_supplicant")) {
> +    mWifiControlThread->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
> +  } else {
> +    mNetUtilControlTrhead->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
> +  }

I'd second vchang's comment that this should be clearer.

::: dom/wifi/WifiProxyService.h
@@ +37,5 @@
>    WifiProxyService();
>    ~WifiProxyService();
>  
>    nsTArray<EventThreadListEntry> mEventThreadList;
> +  nsCOMPtr<nsIThread> mWifiControlThread, mNetUtilControlTrhead;

Nit: Only one member declaration per line, please (i.e. simply repeat the nsCOMPtr<nsIThread> here).
Attachment #8405212 - Flags: feedback?(mrbkap) → feedback+
This is fixed when Henry move the dhcp functionality to network service in Bug 1038531
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.