Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2566423002:
Fix SharedSampler to cope correctly with zero-thread processes. (Closed)
DescriptionFix SharedSampler to cope correctly with zero-thread processes.
SharedSampler previously assumed that all processes have at least one
thread, which appears no longer to be the case for the Windows operating
system's "System and Compressed Memory" process.
BUG=673175
Committed: https://crrev.com/8c4cfd4e82b4465305d60d3830bcbae50076f004
Cr-Commit-Position: refs/heads/master@{#438322}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix >= to > #Messages
Total messages: 23 (14 generated)
The CQ bit was checked by wez@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...
wez@chromium.org changed reviewers: + stanisc@chromium.org
PTAL - minimal fix suitable for merge.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
https://codereview.chromium.org/2566423002/diff/1/chrome/browser/task_manager... File chrome/browser/task_manager/sampling/shared_sampler_win.cc (right): https://codereview.chromium.org/2566423002/diff/1/chrome/browser/task_manager... chrome/browser/task_manager/sampling/shared_sampler_win.cc:460: if (pi->NumberOfThreads >= 0 && This is to prevent underflow when subtracting 1, right? Should this be > 0?
The CQ bit was checked by wez@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.
https://codereview.chromium.org/2566423002/diff/1/chrome/browser/task_manager... File chrome/browser/task_manager/sampling/shared_sampler_win.cc (right): https://codereview.chromium.org/2566423002/diff/1/chrome/browser/task_manager... chrome/browser/task_manager/sampling/shared_sampler_win.cc:460: if (pi->NumberOfThreads >= 0 && On 2016/12/13 17:12:20, stanisc wrote: > This is to prevent underflow when subtracting 1, right? Should this be > 0? *facepalm* Beware late-night coding - fixed! Best of all, Clang actually caught this as a tautologous check (since NumberOfThreads is unsigned). :D
LGTM You still need to get approval from a Task Manager code OWNER - either afakhry@ or nick@.
wez@chromium.org changed reviewers: + nick@chromium.org
+nick@ for TaskManager OWNERS
lgtm
The CQ bit was checked by wez@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": 20001, "attempt_start_ts": 1481669435252910,
"parent_rev": "8925e18bf11ab705c72495e69feb338843ff28e7", "commit_rev":
"a66726b4d69e2fba80d5af67f5ad13d049e67534"}
Message was sent while issue was closed.
Description was changed from ========== Fix SharedSampler to cope correctly with zero-thread processes. SharedSampler previously assumed that all processes have at least one thread, which appears no longer to be the case for the Windows operating system's "System and Compressed Memory" process. BUG=673175 ========== to ========== Fix SharedSampler to cope correctly with zero-thread processes. SharedSampler previously assumed that all processes have at least one thread, which appears no longer to be the case for the Windows operating system's "System and Compressed Memory" process. BUG=673175 Review-Url: https://codereview.chromium.org/2566423002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix SharedSampler to cope correctly with zero-thread processes. SharedSampler previously assumed that all processes have at least one thread, which appears no longer to be the case for the Windows operating system's "System and Compressed Memory" process. BUG=673175 Review-Url: https://codereview.chromium.org/2566423002 ========== to ========== Fix SharedSampler to cope correctly with zero-thread processes. SharedSampler previously assumed that all processes have at least one thread, which appears no longer to be the case for the Windows operating system's "System and Compressed Memory" process. BUG=673175 Committed: https://crrev.com/8c4cfd4e82b4465305d60d3830bcbae50076f004 Cr-Commit-Position: refs/heads/master@{#438322} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8c4cfd4e82b4465305d60d3830bcbae50076f004 Cr-Commit-Position: refs/heads/master@{#438322} |
