Skip to content

Commit

Permalink
Revert of Remove burst mode from VSyncMonitor. (https://codereview.ch…
Browse files Browse the repository at this point in the history
…romium.org/346813002/)

Reason for revert:
Four of the introduced tests time out on Android.

VSyncMonitorTest#testVSyncActivationFromIdleAllowJBVSync
VSyncMonitorTest#testVSyncActivationFromIdleDisallowJBVSync
VSyncMonitorTest#testVSyncPeriodAllowJBVSync
VSyncMonitorTest#testVSyncPeriodDisallowJBVSync

The log output can be seen here:

http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/21188/steps/contentshell_instrumentation_tests/logs/stdio

This failure could be observed in the try-bot results as part of the patch. Please do not use NOTRY unless you're absolutely certain that the failures are unrelated.

Original issue's description:
> Remove burst mode from VSyncMonitor.
> 
> Extra vsyncs after each rendering cause 1% more power consumption.
> 
> NOTRY=true
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279997

TBR=tedchoc@chromium.org,sievers@chromium.org,aruslan@chromium.org,brianderson@chromium.org,skyostil@google.com,skyostil@chromium.org,bulach@chromium.org,miguelg@chromium.org,yfriedman@chromium.org,newt@chromium.org,egor.starkov@samsung.com
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/358473006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280012 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
peter@chromium.org committed Jun 26, 2014
1 parent ca6d162 commit 7deb765
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
1 change: 0 additions & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ Douglas F. Turner <doug.turner@gmail.com>
Eduardo Lima (Etrunko) <eduardo.lima@intel.com>
Edward Crossman <tedoc2000@gmail.com>
Eero Häkkinen <e.hakkinen@partner.samsung.com>
Egor Starkov <egor.starkov@samsung.com>
Ehsan Akhgari <ehsan.akhgari@gmail.com>
Elan Ruusamäe <elan.ruusamae@gmail.com>
Eric Ahn <byungwook.ahn@gmail.com>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public VSyncMonitor call() {
private void performVSyncPeriodTest(boolean enableJBVSync) throws InterruptedException {
// Collect roughly one second of data on a 60 fps display.
collectAndCheckVSync(enableJBVSync, 60, true);
collectAndCheckVSync(enableJBVSync, 60, false);
collectAndCheckVSync(enableJBVSync, VSyncMonitor.MAX_AUTO_ONVSYNC_COUNT, false);
}

private void collectAndCheckVSync(
Expand Down
13 changes: 13 additions & 0 deletions ui/android/java/src/org/chromium/ui/VSyncMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

/**
* Notifies clients of the default displays's vertical sync pulses.
* This class works in "burst" mode: once the update is requested, the listener will be
* called MAX_VSYNC_COUNT times on the vertical sync pulses (on JB) or on every refresh
* period (on ICS, see below), unless stop() is called.
* On ICS, VSyncMonitor relies on setVSyncPointForICS() being called to set a reasonable
* approximation of a vertical sync starting point; see also http://crbug.com/156397.
*/
Expand All @@ -23,6 +26,7 @@ public class VSyncMonitor {
private static final long NANOSECONDS_PER_SECOND = 1000000000;
private static final long NANOSECONDS_PER_MILLISECOND = 1000000;
private static final long NANOSECONDS_PER_MICROSECOND = 1000;
public static final int MAX_AUTO_ONVSYNC_COUNT = 5;

/**
* VSync listener class
Expand All @@ -42,6 +46,7 @@ public interface Listener {
private final long mRefreshPeriodNano;

private boolean mHaveRequestInFlight;
private int mTriggerNextVSyncCount;

// Choreographer is used to detect vsync on >= JB.
private final Choreographer mChoreographer;
Expand Down Expand Up @@ -79,6 +84,7 @@ public VSyncMonitor(Context context, VSyncMonitor.Listener listener, boolean ena
.getDefaultDisplay().getRefreshRate();
if (refreshRate <= 0) refreshRate = 60;
mRefreshPeriodNano = (long) (NANOSECONDS_PER_SECOND / refreshRate);
mTriggerNextVSyncCount = 0;

if (enableJBVSync && Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
// Use Choreographer on JB+ to get notified of vsync.
Expand Down Expand Up @@ -139,13 +145,16 @@ private boolean isVSyncSignalAvailable() {
* after this function is called.
*/
public void stop() {
mTriggerNextVSyncCount = 0;
}

/**
* Request to be notified of the closest display vsync events.
* Listener.onVSync() will be called soon after the upcoming vsync pulses.
* It will be called at most MAX_AUTO_ONVSYNC_COUNT times unless requestUpdate() is called.
*/
public void requestUpdate() {
mTriggerNextVSyncCount = MAX_AUTO_ONVSYNC_COUNT;
postCallback();
}

Expand All @@ -165,6 +174,10 @@ private void onVSyncCallback(long frameTimeNanos, long currentTimeNanos) {
assert mHaveRequestInFlight;
mHaveRequestInFlight = false;
mLastVSyncCpuTimeNano = currentTimeNanos;
if (mTriggerNextVSyncCount >= 0) {
mTriggerNextVSyncCount--;
postCallback();
}
if (mListener != null) {
mListener.onVSync(this, frameTimeNanos / NANOSECONDS_PER_MICROSECOND);
}
Expand Down

0 comments on commit 7deb765

Please sign in to comment.