Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2863553002:
Shape Detection: run Android FaceDetector ops on background thread (Closed)
DescriptionShape Detection: run Android FaceDetector ops on background thread
This CL moves the interactions with android.media.FaceDetector
because they are slow and trigger a "StrictMode policy violation"
(at least in Debug build). Some of the parameters are made final
to use them inside a Runnable.
BUG=718275
Review-Url: https://codereview.chromium.org/2863553002
Cr-Commit-Position: refs/heads/master@{#469393}
Committed: https://chromium.googlesource.com/chromium/src/+/e766fc5ecd017b1ce6cca79c1f4a11a7e883c298
Patch Set 1 : #
Total comments: 2
Patch Set 2 : Use Thread ISO Handler #
Total comments: 3
Patch Set 3 : Use AsyncTask.THREAD_POOL_EXECUTOR.execute ISO a new Thread every time #
Messages
Total messages: 20 (11 generated)
Description was changed from ========== Shape Detection: run Android FaceDetector ops to background thread BUG=659138 ========== to ========== Shape Detection: run Android FaceDetector ops to background thread BUG=718275 ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== Shape Detection: run Android FaceDetector ops to background thread BUG=718275 ========== to ========== Shape Detection: run Android FaceDetector ops on background thread This CL moves the interactions with android.media.FaceDetector because they are slow and trigger a "StrictMode policy violation" (at least in Debug build). Some of the parameters are made final to use them inside a Runnable. BUG=718275 ==========
mcasas@chromium.org changed reviewers: + boliu@chromium.org
boliu@ PTAL
https://codereview.chromium.org/2863553002/diff/20001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java (right): https://codereview.chromium.org/2863553002/diff/20001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:36: mHandler = new Handler(); handler doesn't create a new thread, it just binds to the looper of the thread that it's constructed on. so does this constructor run on a background thread?
Ooops my bad, apologies, PTAL https://codereview.chromium.org/2863553002/diff/20001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java (right): https://codereview.chromium.org/2863553002/diff/20001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:36: mHandler = new Handler(); On 2017/05/04 02:19:40, boliu wrote: > handler doesn't create a new thread, it just binds to the looper of the thread > that it's constructed on. so does this constructor run on a background thread? Argh! Correct, I wanted to use "new Thread".
https://codereview.chromium.org/2863553002/diff/40001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java (right): https://codereview.chromium.org/2863553002/diff/40001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:82: new Thread(new Runnable() { why not use AsyncTask which uses a thread pool? is this really worth creating a brand new short lived thread?
https://codereview.chromium.org/2863553002/diff/40001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java (right): https://codereview.chromium.org/2863553002/diff/40001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:82: new Thread(new Runnable() { On 2017/05/04 04:01:47, boliu wrote: > why not use AsyncTask which uses a thread pool? is this really worth creating a > brand new short lived thread? Or more directly: AsyncTask.THREAD_POOL_EXECUTOR.execute
ptal https://codereview.chromium.org/2863553002/diff/40001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java (right): https://codereview.chromium.org/2863553002/diff/40001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:82: new Thread(new Runnable() { On 2017/05/04 04:15:07, boliu wrote: > On 2017/05/04 04:01:47, boliu wrote: > > why not use AsyncTask which uses a thread pool? is this really worth creating > a > > brand new short lived thread? > > Or more directly: AsyncTask.THREAD_POOL_EXECUTOR.execute Totally reasonable, changed to using the thread pool.
lgtm
The CQ bit was checked by mcasas@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mcasas@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493922484276830,
"parent_rev": "2ecfaf6c232e6e38021aad5df011bfa082116a2c", "commit_rev":
"e766fc5ecd017b1ce6cca79c1f4a11a7e883c298"}
Message was sent while issue was closed.
Description was changed from ========== Shape Detection: run Android FaceDetector ops on background thread This CL moves the interactions with android.media.FaceDetector because they are slow and trigger a "StrictMode policy violation" (at least in Debug build). Some of the parameters are made final to use them inside a Runnable. BUG=718275 ========== to ========== Shape Detection: run Android FaceDetector ops on background thread This CL moves the interactions with android.media.FaceDetector because they are slow and trigger a "StrictMode policy violation" (at least in Debug build). Some of the parameters are made final to use them inside a Runnable. BUG=718275 Review-Url: https://codereview.chromium.org/2863553002 Cr-Commit-Position: refs/heads/master@{#469393} Committed: https://chromium.googlesource.com/chromium/src/+/e766fc5ecd017b1ce6cca79c1f4a... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/e766fc5ecd017b1ce6cca79c1f4a... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
