1184081 - [Boot Performance] sensors_open() takes ~480 ms during boot

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Performance, defect, P4)

Description

[Cervantes Yu [:cyu] [:cervantes]]

During boot, we call sensors_open() on the main thread, which takes about 480 ms. We should move it to the polling thread if it's not required to be called on the main thread.

See the range [5400, 6000] ms of the profile: https://people.mozilla.org/~bgirard/cleopatra/#report=faa95f297fc5a1421c0c1d00e2e3dbb9ac79532c&filter=[{%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A2935,%22end%22%3A9869}]&selection=0,1,149,979,995

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: MozReview Request: Bug 1184081 - Move sensors_open() call off main thread to reduce boot time. r?dhylands

Bug 1184081 - Move sensors_open() call off main thread to reduce boot time. r?dhylands

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #0)
> During boot, we call sensors_open() on the main thread, which takes about
> 480 ms. We should move it to the polling thread if it's not required to be
> called on the main thread.
> 
Forgot to mention that this is on flame-kk. In contrast nexus 5 doesn't have this problem and sensors_open() on it is almost instant. But I think it's good to move it off main thread in case the implementation should ever block, given that calling it off main thread is safe.

Comment 3

[Dave Hylands [:dhylands]]

Comment on attachment 8634651 [details]
MozReview Request: Bug 1184081 - Move sensors_open() call off main thread to reduce boot time. r?dhylands

https://reviewboard.mozilla.org/r/13425/#review12261

::: hal/gonk/GonkSensor.cpp:334
(Diff revision 1)
> +                                                               true));

Why is this posted? If this needs to happen after OpenSensors is called then we need to fix EnableSensorNotifications as well, since it doesn't check to see if OpenSensors has been called, just that the thread was created.

Should we always disapatch SetSensorState to the polling thread? If not, then there could be calls from the main thread and posted calls which are executed in a different order.

Comment 4

[Cervantes Yu [:cyu] [:cervantes]]

https://reviewboard.mozilla.org/r/13425/#review12261

> Why is this posted? If this needs to happen after OpenSensors is called then we need to fix EnableSensorNotifications as well, since it doesn't check to see if OpenSensors has been called, just that the thread was created.
> 
> Should we always disapatch SetSensorState to the polling thread? If not, then there could be calls from the main thread and posted calls which are executed in a different order.

SetSensorState() needs to access sSensorModule, which is initialized in OpenSensors(). The check whether sPollingThread exists also acts as the check whether it's the first time we call EnableSensoeNotifications().

Yes, we should dispatch SetSensorState to the polling thread, but why I am not doing this is that once the polling thread is started, the thread could block in polling the sensor device and the action could be delayed until some sensor responded.

I think what we need to do is creating a control thread for initializing, enabling and disabling the sensors and dispatch to the control requests to that thread.

Comment 5

[Cervantes Yu [:cyu] [:cervantes]]

Just tested on aries-l and sensors_open() also finishes instantly. So we may lower the priority of this bug and have it a nice-to-have improvement.

Comment 6

[Bobby Chien]

[Tracking Requested - why for this release]:

Comment 7

[Cervantes Yu [:cyu] [:cervantes]]

WONTFIX because of bug 1194721.

After the sensor and other code that uses the binary blobs are moved into a separate address space, we won't block on these calls during startup.