Last Comment Bug 734391 - coalesce the device motion events
: coalesce the device motion events
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Geolocation (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla14
Assigned To: Doug Turner (:dougt)
:
Mentors:
Depends on: 1010124 1095867
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-09 08:06 PST by Doug Turner (:dougt)
Modified: 2014-11-07 22:44 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v.1 (8.62 KB, patch)
2012-03-09 08:32 PST, Doug Turner (:dougt)
josh: review+
Details | Diff | Review

Description Doug Turner (:dougt) 2012-03-09 08:06:48 PST
We can improve perf a bit by not sending 3 separate dom events.  Instead, we can coalesce the device motion events by waiting a bit to see if there is additional motion data before posting the dom event.
Comment 1 Doug Turner (:dougt) 2012-03-09 08:32:31 PST
Created attachment 604421 [details] [diff] [review]
patch v.1
Comment 2 Josh Matthews [:jdm] 2012-03-09 09:31:51 PST
Comment on attachment 604421 [details] [diff] [review]
patch v.1

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

r=me

::: dom/system/nsDeviceMotion.cpp
@@ +344,5 @@
>                                     double x,
>                                     double y,
>                                     double z) {
> +  // Attempt to coalesce events
> +  bool fireEvent = false;

Just assign the result of the condition to fireEvent?

@@ +345,5 @@
>                                     double y,
>                                     double z) {
> +  // Attempt to coalesce events
> +  bool fireEvent = false;
> +  if (PR_IntervalNow() > mLastDOMMotionEventTime + PR_MillisecondsToInterval(DEFAULT_SENSOR_POLL))

Let's use TimeStamp::Now() and TimeDuration::FromMilliseconds instead.

@@ +391,5 @@
> +
> +  mLastRotationRate = nsnull;
> +  mLastAccelerationIncluduingGravity = nsnull;
> +  mLastAcceleration = nsnull;
> +  mLastDOMMotionEventTime = PR_IntervalNow();

TimeStamp::Now()

::: dom/system/nsDeviceMotion.h
@@ +98,5 @@
>    virtual void Startup()  = 0;
>    virtual void Shutdown() = 0;
> +
> +  bool mEnabled;
> +  PRIntervalTime mLastDOMMotionEventTime;

TimeStamp
Comment 4 Matt Brubeck (:mbrubeck) 2012-03-14 10:08:57 PDT
Re-landed and merged:
https://hg.mozilla.org/mozilla-central/rev/ed1d52905a61

Note You need to log in before you can comment on or make changes to this bug.