1085760 - Provide a new "Idle" state for nsIIOService

Status: RESOLVED WONTFIX

(Core :: Networking, defect)

Description

[Jason Duell]

Right now nsIOService is either online or offline.  This will add a 3rd state, which is like the old Windows "dial up on demand" stuff.  We'll report nsIIOService.online == true, but the radio will be powered off.  When we get a new request for network activity, we'll try to fire up the radio and fulfill the request.

Not clear what the API looks like yet.  But when we enter this mode, we'll kill all/any existing network connections.  (To be determined: how we'll prevent background apps whose sockets are killed from just firing off new connections. We could possibly delay sending OnStopRequest for killed connections?  Or perhaps upper layers can otherwise engineer things to make apps don't get stuck in this disconnect/reconnect loop).

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

In addition to being able to enter this state, we also need the ability to get a callback whenever someone tries to initiate a network connection so that we can tell the radio code to start attempting connections.

Comment 2

[Jason Duell]

And assuming that the API looks like "return OK from AsyncOpen, but actually stash the request somewhere until the radio code tells us we're OK (or not) to hit the network", we'll need a callback from the radio code, too.

Comment 3

[Josh Matthews [:jdm]]

That sounds exceedingly similar to the network request interception design that I just landed.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to Jason Duell [:jduell] (needinfo? me for lower latency) from comment #2)
> And assuming that the API looks like "return OK from AsyncOpen, but actually
> stash the request somewhere until the radio code tells us we're OK (or not)
> to hit the network", we'll need a callback from the radio code, too.

Kinda. I think the API is "return OK from AsyncOpen, but actually stash the request somewhere until someone tells us we're no longer in Quiescent mode".

Separately I think we need APIs for

* Checking if there's currently any events in the stash
* Registering a callback which is called when the stash goes from empty to non-empty


We *may* also need

* Checking if there are currently any requests in progress
* Registering a callback which is called when the answer to "are there requests in progress" changes

Comment 5

[Jason Duell]

No one can agree on how to pronounce "quiescent" (and Jonas keeps saying "quintessent" :) so we're just going to use "idle".

After some discussion with Jonas and others we have more clarity on how things will work here:

1) when the user hits the button on their phone to turn off the screen, and the phone is configured to use idle mode (not clear if that will be an option, or the default), we'll tell nsIIOService to go into idle mode.  This will be exactly as if .online = false were done, i.e. we'll shut down all network connections.  (Note: another option here if we need it is to set apps offline individually, ie. using the API from bug 786419.  This would allow us to let certain apps keep doing networking for longer, if needed.  It would also allow downloads to finish, since we don't kill divert()'ed channels in the per-app offline API.  Jonas, is there any chance we want to allow downloads to continue in idle mode?)

2) nsIIOService will still report .online == true after this.  Apps will all be frozen (no CPU slice time for them) so mostly we won't get any new network requests and the radio can go to sleep.  But apps that use alarm() will still be woken up and given CPU time, and can launch new necko channels, and when this happens we need to 1) stash the network request somewhere (bikeshed: mIdleStash), 2) wake up the radio/wifi layers and wait for them to report that we're online (one wrinkle here is that we need to make sure we wait to see if wifi is available--apparently it tends to be faster to know that cellular is available, but we don't want to burn users' cell plans if wifi could be used), 3) launch the stashed requests as usual.   
   
4) Open question.  Do we need to do any cleanup/shutdown when alarm() apps are finished with their wakeup session? (IIUC they typically call alarm() again and are put back to sleep).  I see two options here.  1) we just let the network connections for the app finish as normal, with some accelerated connection shutdown times, i.e. we don't keep TCP sockets open for very long after the last HTTP transaction completes on them (usually we keep them for 3 minutes, we'd want shorter so we don't keep the radio alive for 3 mins or wake it back up just to do TCP shutdown handshakes). Similarly if we're using an HTTP proxy we'd want to time out that connection aggressively too if we're the last app awake and using it.  2) We could have the alarm() call notify necko and have necko do nsIOService.SetAppOffline() if we're in idle mode, i.e. we'd aggressively kill off remaining connections.  This would have the advantage that'd we'd kill off websockets/AJAX connections when the app goes back asleep (but not downloads: but I don't expect that background alarm() apps are going to be using the download manager much if at all).  My gut sense is that #2 is both easier to implement and more complete.  Jonas, do you have thoughts here?

Note: alarm() allows a specific wake-up time.  We'll be hopefully moving most apps to a new RequestSync (aka BackgroundSync) API, which improves upon alarm() by waking up apps at the same (imprecise) time so the radio can handle them all at once and stay asleep more of the time.  If we hook alarm() we'll need to hook RequestSync as well.

Comment 7

[Jason Duell]

Jonas: two more things (other than the questions in comment 5): 

1) Can you mark this feature-b2g: ? for me?  (I don't have permission)

2) Can you cc the radio folks so we can start moving on the radio layer bits needed here?

Comment 8

[Honza Bambas (:mayhemer)]

(In reply to Jason Duell [:jduell] (needinfo? me for lower latency) from comment #5)
> No one can agree on how to pronounce "quiescent" (and Jonas keeps saying
> "quintessent" :) so we're just going to use "idle".
> 
> After some discussion with Jonas and others we have more clarity on how
> things will work here:
> 
> 1) when the user hits the button on their phone to turn off the screen, and
> the phone is configured to use idle mode (not clear if that will be an
> option, or the default), we'll tell nsIIOService to go into idle mode.  This
> will be exactly as if .online = false were done, i.e. we'll shut down all
> network connections.  (Note: another option here if we need it is to set
> apps offline individually, ie. using the API from bug 786419.  This would
> allow us to let certain apps keep doing networking for longer, if needed. 
> It would also allow downloads to finish, since we don't kill divert()'ed
> channels in the per-app offline API.  Jonas, is there any chance we want to
> allow downloads to continue in idle mode?)

I definitely want that option or even a default.  Real life example:
- iphone
- mapy.cz (a czech mapping app allowing a full cz+sk map download for complete offline use)
- standing beside a larger city on flaky 3g
- wants to download 350MB of the map data
- iphone or the app (hard to know) does NOT allow the download to proceed when you lock the phone
=> I had to stand as an idiot with my phone in hand keeping it woken for more then 20 minutes ; we definitely want to be smarter :D


> 
> 2) nsIIOService will still report .online == true after this.  

Hmm.. but existing connection will break, so apps should be notified with OnStop IMO.  We cannot pretend we still keep the conns open, right?


> Apps will all
> be frozen (no CPU slice time for them) so mostly we won't get any new
> network requests and the radio can go to sleep.  But apps that use alarm()
> will still be woken up and given CPU time, and can launch new necko
> channels, and when this happens we need to 1) stash the network request
> somewhere (bikeshed: mIdleStash), 2) wake up the radio/wifi layers and wait
> for them to report that we're online (one wrinkle here is that we need to
> make sure we wait to see if wifi is available--apparently it tends to be
> faster to know that cellular is available, but we don't want to burn users'
> cell plans if wifi could be used), 3) launch the stashed requests as usual. 
> 

This sounds like the windows dialer code to me, as you've already mentioned once.  Why don't we just step a side during build-up/connection of the socket and wait with actual call to PR_Connect until RIL tells us "we have a wifi".  I don;t think anything needs to be done so high as "AsyncOpen" as you are still talking about.  Also, don't forget we have to inject captive portal detection in there as well probably..

Hence, I don't think we need any mIdleStash arrays.

>    
> 4) Open question.  Do we need to do any cleanup/shutdown when alarm() apps
> are finished with their wakeup session? (IIUC they typically call alarm()
> again and are put back to sleep).  I see two options here.  1) we just let
> the network connections for the app finish as normal, with some accelerated
> connection shutdown times, i.e. we don't keep TCP sockets open for very long
> after the last HTTP transaction completes on them (usually we keep them for
> 3 minutes, we'd want shorter so we don't keep the radio alive for 3 mins or
> wake it back up just to do TCP shutdown handshakes). Similarly if we're
> using an HTTP proxy we'd want to time out that connection aggressively too
> if we're the last app awake and using it.  2) We could have the alarm() call
> notify necko and have necko do nsIOService.SetAppOffline() if we're in idle
> mode, i.e. we'd aggressively kill off remaining connections.  This would
> have the advantage that'd we'd kill off websockets/AJAX connections when the
> app goes back asleep (but not downloads: but I don't expect that background
> alarm() apps are going to be using the download manager much if at all).  My
> gut sense is that #2 is both easier to implement and more complete.  Jonas,
> do you have thoughts here?

I like #2 as well more.

Comment 9

[Jason Duell]

> existing connection will break, so apps should be notified with OnStop IMO

Yes, when we kill all network connections we will deliver OnStop to all connections (though IIUC most apps won't receive the OnStop event until idle mode is exited, because they get no CPU time)

Comment 10

[Jason Duell]

Jonas tells me we should indeed kill an app's remaining necko channels when it wakes up from alarm() and then goes back to sleep with another alarm() call.  So we want solution #2 from part 4 of comment 5. (ie. have the alarm() call notify necko and have necko do nsIOService.SetAppOffline() if we're in idle mode).

Jonas, I still need you to do the stuff in comment 7.

Comment 11

[Ethan Tseng [:ethan]]

Hi Jason,

"quiescent" is hard to pronounce but really a cool name. :p

I think the intent of this bug to optimize power consumption of Wi-Fi (correct me if I'm wrong).
As you are investigating the improvement plan from Necko, we had a discussion in Taipei today.
We looked at the problem from the perspective of Wi-Fi driver/chip, and discussed another approach
by exploiting the Power Save Mode specified by 802.11.

AFAIK, AOSP always enables Power Save Mode, (although this can be altered by user preference),
which could dramatically reduce power consumption compared to normal mode (Full Power Mode).
(Our device team has some measurement data, I think they can share if you're interested in).
And AOSP only change to Full Power Mode when needed, such as running DHCP, because Power Save Mode
doesn't accept Wi-Fi multicast/broadcast frames.

Currently FXOS uses Power Save Mode only when the phone enter sleep mode and no app holds wifiLock.
Maybe we can re-design Wi-Fi component to exploit the Power Save Mode.

We have a draft design. Please refer to this diagram:
https://wiki.mozilla.org/images/c/c6/WifiPowerModeState.svg

This is a preliminary design and is for your reference.
We guess if it works, maybe Necko doesn't have to bother controlling Wi-Fi.

What do you think?

Comment 12

[Adam Rogers (:arog)]

I've set the feature-b2g flag

Vincent, as per the request in comment 7, can you help cc the right WiFi people on this bug please?

Comment 13

[Vincent Chang[:vchang][changyihsin]]

Henry and Ethan are the right persons to join the discussion there.

Comment 14

[Adam Rogers (:arog)]

What impact if any do you feel this will have on push connections?

Comment 15

[Jason Duell]

Adam: assuming push connections are using TCP (and not some separate cellular connectivity that can remain powered up somehow while the necko stack is idle), the phone won't get any push notifications while it's idle.   I'm assuming the push code is written so that it will reconnect automatically when the network goes back up (and hopefully the server stores any messages that were "missed" and sends them then).  But I don't know our push stack very well--doug turner (dougt) might have answers for you.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Adam, see comment 7.

Comment 17

[Adam Rogers (:arog)]

Jonas, Done. 

Jason, Thanks let me dig into this a bit.

Comment 18

[Ethan Tseng [:ethan]]

Attachment: SequenceDiagramOfIdleState.svg

We had a draft design of the new Service idle state.
Service.io.idle = {true | false}

This sequence diagram depicting how Necko and System interact when the
new idle state is introduced.

Comment 19

[Henry Chang [:hchang]]

(In reply to Ethan Tseng [:ethan] from comment #18)
> Created attachment 8525134 [details]
> SequenceDiagramOfIdleState.svg
> 
> We had a draft design of the new Service idle state.
> Service.io.idle = {true | false}
> 
> This sequence diagram depicting how Necko and System interact when the
> new idle state is introduced.

Thanks Ethan for making this awesome diagram!

Comment 20

[Ethan Tseng [:ethan]]

Hi Jason,

Let me briefly explain the idea of our draft design.

The design introduces a new interface setIdle(bool) to Necko.
It could be called by Necko itself or other monitor service when either of them detects that there is
no underlying activity.
Necko notifies the System app to turn off/on Wi-Fi and mobile network through an idle/activeEvent.

There are three points we want to emphasize in our design:
1. The power save policy
   This policy decides whether the system should enter Power Save mode or shutdown Wi-Fi.
   It is wrapped in the System app.

2. Who transitions to idle state?
   Necko is responsible to make this transition. Once idled, any network request should fail or be 
   stashed according to Necko's internal design.

3. Who transitions to non-idle state?
   System makes this transition after it receives an activeEvent and successfully turns on network.

Comment 21

[Honza Bambas (:mayhemer)]

(In reply to Ethan Tseng [:ethan] from comment #20)
> Hi Jason,
> 
> Let me briefly explain the idea of our draft design.
> 
> The design introduces a new interface setIdle(bool) to Necko.
> It could be called by Necko itself or other monitor service when either of
> them detects that there is
> no underlying activity.

Please define exactly "no underlying activity"

> Necko notifies the System app to turn off/on Wi-Fi and mobile network
> through an idle/activeEvent.
> 
> There are three points we want to emphasize in our design:
> 1. The power save policy
>    This policy decides whether the system should enter Power Save mode or
> shutdown Wi-Fi.
>    It is wrapped in the System app.
> 
> 2. Who transitions to idle state?
>    Necko is responsible to make this transition. Once idled, any network
> request should fail or be 
>    stashed according to Necko's internal design.
> 
> 3. Who transitions to non-idle state?
>    System makes this transition after it receives an activeEvent and
> successfully turns on network.

This is like the windows dialer.  Exactly the same case.  Stashing is the way here.

Comment 22

[Henry Chang [:hchang]]

(In reply to Honza Bambas (:mayhemer) from comment #21)
> (In reply to Ethan Tseng [:ethan] from comment #20)
> > Hi Jason,
> > 
> > Let me briefly explain the idea of our draft design.
> > 
> > The design introduces a new interface setIdle(bool) to Necko.
> > It could be called by Necko itself or other monitor service when either of
> > them detects that there is
> > no underlying activity.
> 
> Please define exactly "no underlying activity"
> 
> > Necko notifies the System app to turn off/on Wi-Fi and mobile network
> > through an idle/activeEvent.
> > 
> > There are three points we want to emphasize in our design:
> > 1. The power save policy
> >    This policy decides whether the system should enter Power Save mode or
> > shutdown Wi-Fi.
> >    It is wrapped in the System app.
> > 
> > 2. Who transitions to idle state?
> >    Necko is responsible to make this transition. Once idled, any network
> > request should fail or be 
> >    stashed according to Necko's internal design.
> > 
> > 3. Who transitions to non-idle state?
> >    System makes this transition after it receives an activeEvent and
> > successfully turns on network.
> 
> This is like the windows dialer.  Exactly the same case.  Stashing is the
> way here.

Hi :mayhemer,

We haven't had an idea of the best definition "no underlying activity" 
but it's supposed to be a state where we can set to idle mode with 
less worries. For example, no network traffic for 5 minutes. We are 
also curious the timing that we should set to the idle mode when 
this bug was created. Following might be some good triggers:

1) Screen off for a long while
2) Very low battery
3) No network traffic for like 10 minutes

Thanks!

Comment 23

[Jason Duell]

It sounds like we want to proceed for now with the architecture we have for "idle" mode (now I'm thinking we should call it "sleep" mode :) and we can figure out later when exactly it will get triggered and whether there will be an intermediate "wifi-low-power" mode as well.

Comment 24

[Daniel Stenberg [:bagder]]

Another vote for "sleep mode", it seems more appropriate from the network code point of view.

The way I see it, it is basically technically offline mode but claiming it is online if asked, and coming back from sleep mode is getting back online again (for real). Neither online nor offline events should be sent for that but a network change event should be issued coming back from sleep mode I guess.

Of course, if someone actually tries to do a network operation while in sleep mode, we might have reason to treat that differently than in real offline mode.

Comment 25

[Jason Duell]

In portland, we talked to the B2G wifi hardware guys, and the architecture here is pretty much correct.  We realized that we'll need a callback function

   nsIIOService.SetIdle(true, listener) // tells necko to shutdown all TCP/UDP sockets.
   listener.NeckoIdle()                 // tells listener that shutdown is done

so we can do TCP shutdown handshakes before we actually power off the wifi/cellular hardware.

The B2G folks will open a new bug for some call like

   NetworkHardware.WakeUp()    // power up cellular and wifi

It's ok if this is two separate functions (for cell and for wifi), or a single one.  We were also thinking that instead of a listener we could use an nsIObserverService notification ('wifi-up', etc), as other components are likely to be interested in it.

Comment 26

[Jason Duell]

   nsIIOService.SetIdle(true, listener) // tells necko to shutdown all TCP/UDP sockets.
   listener.NeckoIdle()                 // tells listener that shutdown is done

Also, we mentioned that the wifi hardware layer will hopefully be smart enough to know that the TCP shutdown packets are not "new traffic".  I.e. if 'sleep' mode happens only after N minutes of network inactivity, we shouldn't restart that clock for TCP shutdown traffic (but we would for a new network connection?).  If it's hard to distinguish traffic like this at the hardware layer, we could also fire off an nsIObserverService 'network-idle' notification any time we have no open TCP/UDP sockets.  So we could omit the 'listener' argument to setIdle().

Comment 27

[Kai-Chih Hu [:khu]]

This bug isn't included in 2.2 proposed scope. Suggest to remove 2.2+. Ping Adam to see if there is any concern. Thanks.

Comment 28

[Adam Rogers (:arog)]

Agreed.

Comment 29

[Patrick McManus [:mcmanus]]

wontfix most of this as b2g work, but the battery friendly scheduler will provide much of the same funcationality with less complexity