Add metrics to track performance of important steps in saving an offline page |
||||||||
Issue descriptionWhen saving a user requested offline page there are 3 steps involved: 1) Saving a snapshot of the page into an MHTML files in a private folder. 2) Computing the hash of the saved file. 3) Moving the file to the final downloads destination. We intend to improve the robustness of this operation in the near future. So we want to collect performance metrics that will be used to assess any regressions that might come with those changes.
,
Nov 29
To note: we already have the overall time taken to save a page, broken down per namespace, in the OfflinePages.SavePageTime.<namespace> metric. We still need to add metrics to the internal steps listed above.
,
Nov 29
Adding to #2: that metric is is only reported on successful saves though.
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a816a03231507d185c23f974da14f0bab688964 commit 9a816a03231507d185c23f974da14f0bab688964 Author: Carlos Knippschild <carlosk@chromium.org> Date: Wed Dec 05 22:54:53 2018 Add metrics for internal steps of saving an offline page Adds new metrics to track the time taken by each of 4 internal steps executed to save an offline page: 1) Saving a snapshot of the the web page as an archive (MHTML file). 2) Computing the hash/digest of the saved file. 3) Moving the file to the public downloads folder. 4) Creating the metadata entry in the offline pages store. These metrics are only reported when their respective steps are successfully completed, even if the full save operation fails. Also note that step 3) is performed only when saving user requested pages. It also fixes a bug in OfflinePageTestArchiver where it was not properly returning the results of a PublishArchive call. Bug: 909966 Change-Id: I31664f813cb193ce746874120c34b30f87ae0e7f Reviewed-on: https://chromium-review.googlesource.com/c/1356219 Commit-Queue: Carlos Knippschild <carlosk@chromium.org> Reviewed-by: Jian Li <jianli@chromium.org> Reviewed-by: Robert Kaplow <rkaplow@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Cr-Commit-Position: refs/heads/master@{#614151} [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/chrome/browser/offline_pages/offline_page_mhtml_archiver.cc [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/chrome/browser/offline_pages/offline_page_mhtml_archiver.h [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/chrome/browser/offline_pages/offline_page_mhtml_archiver_unittest.cc [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/model/offline_page_model_taskified.cc [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/model/offline_page_model_taskified.h [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/offline_page_archiver.cc [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/offline_page_archiver.h [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/offline_page_test_archiver.cc [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/components/offline_pages/core/offline_page_test_archiver.h [modify] https://crrev.com/9a816a03231507d185c23f974da14f0bab688964/tools/metrics/histograms/histograms.xml
,
Dec 5
The whole change only added a few metrics reporting. So it seems to be safe for beta merging.
,
Dec 7
I'd like to request merging of commit 9a816a03231507d185c23f974da14f0bab688964 into the M72 branch. It was submitted with tests assuring the metrics are properly reported. jianli@ also vouched for it being a safe change. I verified the metrics are correctly reported in Canary 73.0.3632.4.
,
Dec 8
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 10
Pls merge you change to M72 branch 3626 ASAP so we can pick it up for next Dev/Beta release, RC cut on Monday, 12/10 @ 1:00 PM PT. Thank you.
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c2a952d349b8efb06821e12982425dcfaca9a24 commit 6c2a952d349b8efb06821e12982425dcfaca9a24 Author: Carlos Knippschild <carlosk@chromium.org> Date: Mon Dec 10 19:10:31 2018 Add metrics for internal steps of saving an offline page (M72 Merge) Adds new metrics to track the time taken by each of 4 internal steps executed to save an offline page: 1) Saving a snapshot of the the web page as an archive (MHTML file). 2) Computing the hash/digest of the saved file. 3) Moving the file to the public downloads folder. 4) Creating the metadata entry in the offline pages store. These metrics are only reported when their respective steps are successfully completed, even if the full save operation fails. Also note that step 3) is performed only when saving user requested pages. It also fixes a bug in OfflinePageTestArchiver where it was not properly returning the results of a PublishArchive call. Bug: 909966 Change-Id: I31664f813cb193ce746874120c34b30f87ae0e7f Reviewed-on: https://chromium-review.googlesource.com/c/1356219 Commit-Queue: Carlos Knippschild <carlosk@chromium.org> Reviewed-by: Jian Li <jianli@chromium.org> Reviewed-by: Robert Kaplow <rkaplow@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614151}(cherry picked from commit 9a816a03231507d185c23f974da14f0bab688964) Reviewed-on: https://chromium-review.googlesource.com/c/1370390 Reviewed-by: Dan H <harringtond@google.com> Cr-Commit-Position: refs/branch-heads/3626@{#225} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/chrome/browser/offline_pages/offline_page_mhtml_archiver.cc [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/chrome/browser/offline_pages/offline_page_mhtml_archiver.h [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/chrome/browser/offline_pages/offline_page_mhtml_archiver_unittest.cc [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/model/offline_page_model_taskified.cc [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/model/offline_page_model_taskified.h [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/offline_page_archiver.cc [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/offline_page_archiver.h [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/offline_page_test_archiver.cc [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/components/offline_pages/core/offline_page_test_archiver.h [modify] https://crrev.com/6c2a952d349b8efb06821e12982425dcfaca9a24/tools/metrics/histograms/histograms.xml
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c2a952d349b8efb06821e12982425dcfaca9a24 Commit: 6c2a952d349b8efb06821e12982425dcfaca9a24 Author: carlosk@chromium.org Commiter: harringtond@google.com Date: 2018-12-10 19:10:31 +0000 UTC Add metrics for internal steps of saving an offline page (M72 Merge) Adds new metrics to track the time taken by each of 4 internal steps executed to save an offline page: 1) Saving a snapshot of the the web page as an archive (MHTML file). 2) Computing the hash/digest of the saved file. 3) Moving the file to the public downloads folder. 4) Creating the metadata entry in the offline pages store. These metrics are only reported when their respective steps are successfully completed, even if the full save operation fails. Also note that step 3) is performed only when saving user requested pages. It also fixes a bug in OfflinePageTestArchiver where it was not properly returning the results of a PublishArchive call. Bug: 909966 Change-Id: I31664f813cb193ce746874120c34b30f87ae0e7f Reviewed-on: https://chromium-review.googlesource.com/c/1356219 Commit-Queue: Carlos Knippschild <carlosk@chromium.org> Reviewed-by: Jian Li <jianli@chromium.org> Reviewed-by: Robert Kaplow <rkaplow@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614151}(cherry picked from commit 9a816a03231507d185c23f974da14f0bab688964) Reviewed-on: https://chromium-review.googlesource.com/c/1370390 Reviewed-by: Dan H <harringtond@google.com> Cr-Commit-Position: refs/branch-heads/3626@{#225} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Jan 4
,
Jan 4
,
Jan 4
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by carl...@google.com
, Nov 29