Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2811393002:
Log UpdateJumpList() and CreateIconFiles() execution time to UMA (Closed)
DescriptionLog UpdateJumpList() and CreateIconFiles() execution time to UMA
The separation of JumpListIcons delete tasks and update tasks as in
crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList
method) is now the major contributor of the overall long execution time.
So it's time to investigate the subtasks in RunUpdateJumpList().
The count of the new icon files that will be created per update is also
logged to UMA.
BUG=40407, 179576
Review-Url: https://codereview.chromium.org/2811393002
Cr-Commit-Position: refs/heads/master@{#464510}
Committed: https://chromium.googlesource.com/chromium/src/+/6f797ffc358c069eb954a59ea811c12d80208feb
Patch Set 1 #Patch Set 2 : Log the count of the new icon files created to UMA. #
Total comments: 2
Patch Set 3 : Better UMA metric names #
Total comments: 2
Patch Set 4 : Add TODO comments #Patch Set 5 : Merge branch 'master' of https://chromium.googlesource.com/chromium/src into lookintoupdatejumplist #Messages
Total messages: 35 (29 generated)
The CQ bit was checked by chengx@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...
Description was changed from ========== Log more jumplist update data to UMA BUG=40407, 179576 ========== to ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The counts of local_most_visited_pages and local_recently_closed_pages are also logged to UMA. BUG=40407, 179576 ==========
Description was changed from ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The counts of local_most_visited_pages and local_recently_closed_pages are also logged to UMA. BUG=40407, 179576 ========== to ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The counts of local_most_visited_pages and local_recently_closed_pages are also logged to UMA. BUG=40407, 179576 ==========
Description was changed from ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The counts of local_most_visited_pages and local_recently_closed_pages are also logged to UMA. BUG=40407, 179576 ========== to ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The counts of local_most_visited_pages and local_recently_closed_pages are also logged to UMA. BUG=40407, 179576 ==========
chengx@chromium.org changed reviewers: + gab@chromium.org, isherman@chromium.org
PTAL at this small change. Thanks! gab@ for jumplist.cc isherman@ for histograms.xml
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The counts of local_most_visited_pages and local_recently_closed_pages are also logged to UMA. BUG=40407, 179576 ========== to ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The count of the new icon files that will be created per update is also logged to UMA. BUG=40407, 179576 ==========
The CQ bit was checked by chengx@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.
Metrics LGTM % a nit: https://codereview.chromium.org/2811393002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2811393002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:81470: +<histogram name="WinJumplist.CreateIconFiles" units="ms"> Optional nit: I'd append "Duration" to this histogram name and the UpdateJumpList one.
The CQ bit was checked by chengx@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: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by chengx@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.
lgtm https://codereview.chromium.org/2811393002/diff/40001/chrome/browser/win/jump... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2811393002/diff/40001/chrome/browser/win/jump... chrome/browser/win/jumplist.cc:124: SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.CreateIconFilesDuration"); Add // TODO(chengx): Remove after settling http://crbug.com/40407. on all of these.
The CQ bit was checked by chengx@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.
All the comments are addressed in the new patch set. Thanks! https://codereview.chromium.org/2811393002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2811393002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:81470: +<histogram name="WinJumplist.CreateIconFiles" units="ms"> On 2017/04/12 23:33:34, Ilya Sherman wrote: > Optional nit: I'd append "Duration" to this histogram name and the > UpdateJumpList one. Done. https://codereview.chromium.org/2811393002/diff/40001/chrome/browser/win/jump... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2811393002/diff/40001/chrome/browser/win/jump... chrome/browser/win/jumplist.cc:124: SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.CreateIconFilesDuration"); On 2017/04/13 15:14:22, gab wrote: > Add > // TODO(chengx): Remove after settling http://crbug.com/40407. > on all of these. Done.
The CQ bit was checked by chengx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from isherman@chromium.org, gab@chromium.org Link to the patchset: https://codereview.chromium.org/2811393002/#ps80001 (title: "Merge branch 'master' of https://chromium.googlesource.com/chromium/src into lookintoupdatejumplist")
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": 80001, "attempt_start_ts": 1492111325059540,
"parent_rev": "aee5ec5b3423c48e79c296a054636d60473cfa8d", "commit_rev":
"6f797ffc358c069eb954a59ea811c12d80208feb"}
Message was sent while issue was closed.
Description was changed from ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The count of the new icon files that will be created per update is also logged to UMA. BUG=40407, 179576 ========== to ========== Log UpdateJumpList() and CreateIconFiles() execution time to UMA The separation of JumpListIcons delete tasks and update tasks as in crrev.com/2807883002 indicates the update task (i.e., RunUpdateJumpList method) is now the major contributor of the overall long execution time. So it's time to investigate the subtasks in RunUpdateJumpList(). The count of the new icon files that will be created per update is also logged to UMA. BUG=40407, 179576 Review-Url: https://codereview.chromium.org/2811393002 Cr-Commit-Position: refs/heads/master@{#464510} Committed: https://chromium.googlesource.com/chromium/src/+/6f797ffc358c069eb954a59ea811... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/6f797ffc358c069eb954a59ea811... |
