Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Move all interaction with vibrator HAL outside VibrationThread lock" into sc-dev

Browse Source
This commit is contained in:
Lais Andrade
2021-06-01 16:34:00 +00:00
committed by Android (Google) Code Review
parent 52109a9328 9934c33d8c
commit 79ce03a8de
2 changed files with 119 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

110
services/core/java/com/android/server/vibrator/VibrationThread.java
View File

@@ -45,8 +45,10 @@ import com.android.internal.util.FrameworkStatsLog;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.PriorityQueue; import java.util.PriorityQueue;
import java.util.Queue;
/** Plays a {@link Vibration} in dedicated thread. */ /** Plays a {@link Vibration} in dedicated thread. */
final class VibrationThread extends Thread implements IBinder.DeathRecipient { final class VibrationThread extends Thread implements IBinder.DeathRecipient {
@@ -171,7 +173,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
Slog.d(TAG, "Synced vibration complete reported by vibrator manager"); Slog.d(TAG, "Synced vibration complete reported by vibrator manager");
} }
for (int i = 0; i < mVibrators.size(); i++) { for (int i = 0; i < mVibrators.size(); i++) {
mStepQueue.consumeOnVibratorComplete(mVibrators.keyAt(i)); mStepQueue.notifyVibratorComplete(mVibrators.keyAt(i));
} }
mLock.notify(); mLock.notify();
} }
@@ -183,7 +185,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
if (DEBUG) { if (DEBUG) {
Slog.d(TAG, "Vibration complete reported by vibrator " + vibratorId); Slog.d(TAG, "Vibration complete reported by vibrator " + vibratorId);
} }
mStepQueue.consumeOnVibratorComplete(vibratorId); mStepQueue.notifyVibratorComplete(vibratorId);
mLock.notify(); mLock.notify();
} }
} }
@@ -192,26 +194,25 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "playVibration"); Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "playVibration");
try { try {
CombinedVibration.Sequential effect = toSequential(mVibration.getEffect()); CombinedVibration.Sequential effect = toSequential(mVibration.getEffect());
mStepQueue.offer(new StartVibrateStep(effect));
int stepsPlayed = 0; int stepsPlayed = 0;
while (!mStepQueue.isEmpty()) {
synchronized (mLock) { long waitTime = mStepQueue.calculateWaitTime();
mStepQueue.offer(new StartVibrateStep(effect)); if (waitTime <= 0) {
Step topOfQueue; stepsPlayed += mStepQueue.consumeNext();
} else {
while ((topOfQueue = mStepQueue.peek()) != null) { synchronized (mLock) {
long waitTime = topOfQueue.calculateWaitTime();
if (waitTime <= 0) {
stepsPlayed += mStepQueue.consume();
} else {
try { try {
mLock.wait(waitTime); mLock.wait(waitTime);
} catch (InterruptedException e) { } } catch (InterruptedException e) {
} }
if (mForceStop) {
mStepQueue.cancel();
return Vibration.Status.CANCELLED;
} }
} }
if (mForceStop) {
mStepQueue.cancel();
return Vibration.Status.CANCELLED;
}
} }
// Some effects might be ignored because the specified vibrator don't exist or doesn't // Some effects might be ignored because the specified vibrator don't exist or doesn't
@@ -295,54 +296,75 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
private final class StepQueue { private final class StepQueue {
@GuardedBy("mLock") @GuardedBy("mLock")
private final PriorityQueue<Step> mNextSteps = new PriorityQueue<>(); private final PriorityQueue<Step> mNextSteps = new PriorityQueue<>();
@GuardedBy("mLock") @GuardedBy("mLock")
private final Queue<Step> mPendingOnVibratorCompleteSteps = new LinkedList<>();
public void offer(@NonNull Step step) { public void offer(@NonNull Step step) {
mNextSteps.offer(step); synchronized (mLock) {
mNextSteps.offer(step);
}
} }
@GuardedBy("mLock") public boolean isEmpty() {
@Nullable synchronized (mLock) {
public Step peek() { return mPendingOnVibratorCompleteSteps.isEmpty() && mNextSteps.isEmpty();
return mNextSteps.peek(); }
}
/** Returns the time in millis to wait before calling {@link #consumeNext()}. */
public long calculateWaitTime() {
Step nextStep;
synchronized (mLock) {
if (!mPendingOnVibratorCompleteSteps.isEmpty()) {
// Steps anticipated by vibrator complete callback should be played right away.
return 0;
}
nextStep = mNextSteps.peek();
}
return nextStep == null ? 0 : nextStep.calculateWaitTime();
} }
/** /**
* Play and remove the step at the top of this queue, and also adds the next steps * Play and remove the step at the top of this queue, and also adds the next steps generated
* generated to be played next. * to be played next.
* *
* @return the number of steps played * @return the number of steps played
*/ */
@GuardedBy("mLock") public int consumeNext() {
public int consume() { Step nextStep = pollNext();
Step nextStep = mNextSteps.poll();
if (nextStep != null) { if (nextStep != null) {
mNextSteps.addAll(nextStep.play()); // This might turn on the vibrator and have a HAL latency. Execute this outside any
// lock to avoid blocking other interactions with the thread.
List<Step> nextSteps = nextStep.play();
synchronized (mLock) {
mNextSteps.addAll(nextSteps);
}
return 1; return 1;
} }
return 0; return 0;
} }
/** /**
* Play and remove the step in this queue that should be anticipated by the vibrator * Notify the step in this queue that should be anticipated by the vibrator completion
* completion callback. * callback and keep it separate to be consumed by {@link #consumeNext()}.
*
* <p>This is a lightweight method that do not trigger any operation from {@link
* VibratorController}, so it can be called directly from a native callback.
* *
* <p>This assumes only one of the next steps is waiting on this given vibrator, so the * <p>This assumes only one of the next steps is waiting on this given vibrator, so the
* first step found is played by this method, in no particular order. * first step found will be anticipated by this method, in no particular order.
*/ */
@GuardedBy("mLock") @GuardedBy("mLock")
public void consumeOnVibratorComplete(int vibratorId) { public void notifyVibratorComplete(int vibratorId) {
Iterator<Step> it = mNextSteps.iterator(); Iterator<Step> it = mNextSteps.iterator();
List<Step> nextSteps = EMPTY_STEP_LIST;
while (it.hasNext()) { while (it.hasNext()) {
Step step = it.next(); Step step = it.next();
if (step.shouldPlayWhenVibratorComplete(vibratorId)) { if (step.shouldPlayWhenVibratorComplete(vibratorId)) {
it.remove(); it.remove();
nextSteps = step.play(); mPendingOnVibratorCompleteSteps.offer(step);
break; break;
} }
} }
mNextSteps.addAll(nextSteps);
} }
/** /**
@@ -350,13 +372,25 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
* *
* <p>This will remove and trigger {@link Step#cancel()} in all steps, in order. * <p>This will remove and trigger {@link Step#cancel()} in all steps, in order.
*/ */
@GuardedBy("mLock")
public void cancel() { public void cancel() {
Step step; Step step;
while ((step = mNextSteps.poll()) != null) { while ((step = pollNext()) != null) {
// This might turn off the vibrator and have a HAL latency. Execute this outside
// any lock to avoid blocking other interactions with the thread.
step.cancel(); step.cancel();
} }
} }
@Nullable
private Step pollNext() {
synchronized (mLock) {
// Prioritize the steps anticipated by a vibrator complete callback.
if (!mPendingOnVibratorCompleteSteps.isEmpty()) {
return mPendingOnVibratorCompleteSteps.poll();
}
return mNextSteps.poll();
}
}
} }
/** /**

56
services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
View File

@@ -85,12 +85,17 @@ public class VibrationThreadTest {
private static final String PACKAGE_NAME = "package"; private static final String PACKAGE_NAME = "package";
private static final VibrationAttributes ATTRS = new VibrationAttributes.Builder().build(); private static final VibrationAttributes ATTRS = new VibrationAttributes.Builder().build();
@Rule public MockitoRule mMockitoRule = MockitoJUnit.rule(); @Rule
public MockitoRule mMockitoRule = MockitoJUnit.rule();
@Mock private VibrationThread.VibrationCallbacks mThreadCallbacks; @Mock
@Mock private VibratorController.OnVibrationCompleteListener mControllerCallbacks; private VibrationThread.VibrationCallbacks mThreadCallbacks;
@Mock private IBinder mVibrationToken; @Mock
@Mock private IBatteryStats mIBatteryStatsMock; private VibratorController.OnVibrationCompleteListener mControllerCallbacks;
@Mock
private IBinder mVibrationToken;
@Mock
private IBatteryStats mIBatteryStatsMock;
private final Map<Integer, FakeVibratorControllerProvider> mVibratorProviders = new HashMap<>(); private final Map<Integer, FakeVibratorControllerProvider> mVibratorProviders = new HashMap<>();
private PowerManager.WakeLock mWakeLock; private PowerManager.WakeLock mWakeLock;
@@ -253,7 +258,7 @@ public class VibrationThreadTest {
Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); Thread cancellingThread = new Thread(() -> vibrationThread.cancel());
cancellingThread.start(); cancellingThread.start();
waitForCompletion(vibrationThread, 20); waitForCompletion(vibrationThread, /* timeout= */ 50);
waitForCompletion(cancellingThread); waitForCompletion(cancellingThread);
verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));
@@ -278,7 +283,7 @@ public class VibrationThreadTest {
Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); Thread cancellingThread = new Thread(() -> vibrationThread.cancel());
cancellingThread.start(); cancellingThread.start();
waitForCompletion(vibrationThread, 20); waitForCompletion(vibrationThread, /* timeout= */ 50);
waitForCompletion(cancellingThread); waitForCompletion(cancellingThread);
verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));
@@ -844,6 +849,39 @@ public class VibrationThreadTest {
delay < maxDelay); delay < maxDelay);
} }
@LargeTest
@Test
public void vibrate_cancelSlowVibrator_cancelIsNotBlockedByVibrationThread() throws Exception {
FakeVibratorControllerProvider fakeVibrator = mVibratorProviders.get(VIBRATOR_ID);
fakeVibrator.setSupportedEffects(VibrationEffect.EFFECT_CLICK);
long latency = 5_000; // 5s
fakeVibrator.setLatency(latency);
long vibrationId = 1;
VibrationEffect effect = VibrationEffect.get(VibrationEffect.EFFECT_CLICK);
VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect);
assertTrue(waitUntil(
t -> !fakeVibrator.getEffectSegments().isEmpty(),
vibrationThread, TEST_TIMEOUT_MILLIS));
assertTrue(vibrationThread.isAlive());
// Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should
// fail at waitForCompletion(cancellingThread).
Thread cancellingThread = new Thread(() -> vibrationThread.cancel());
cancellingThread.start();
// Cancelling the vibration should be fast and return right away, even if the thread is
// stuck at the slow call to the vibrator.
waitForCompletion(cancellingThread, /* timeout= */ 50);
// After the vibrator call ends the vibration is cancelled and the vibrator is turned off.
waitForCompletion(vibrationThread, /* timeout= */ latency + TEST_TIMEOUT_MILLIS);
verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));
assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating());
}
@Test @Test
public void vibrate_multiplePredefinedCancel_cancelsVibrationImmediately() throws Exception { public void vibrate_multiplePredefinedCancel_cancelsVibrationImmediately() throws Exception {
mockVibrators(1, 2); mockVibrators(1, 2);
@@ -870,7 +908,7 @@ public class VibrationThreadTest {
Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); Thread cancellingThread = new Thread(() -> vibrationThread.cancel());
cancellingThread.start(); cancellingThread.start();
waitForCompletion(vibrationThread, 20); waitForCompletion(vibrationThread, /* timeout= */ 50);
waitForCompletion(cancellingThread); waitForCompletion(cancellingThread);
verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));
@@ -902,7 +940,7 @@ public class VibrationThreadTest {
Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); Thread cancellingThread = new Thread(() -> vibrationThread.cancel());
cancellingThread.start(); cancellingThread.start();
waitForCompletion(vibrationThread, 20); waitForCompletion(vibrationThread, /* timeout= */ 50);
waitForCompletion(cancellingThread); waitForCompletion(cancellingThread);
verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));