Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2467763002:
[loading metric] filter out events during cache warming (Closed)
|
Created:
4 years, 1 month ago by Kunihiko Sakamoto Modified:
4 years, 1 month ago CC:
catapult-reviews_chromium.org, tracing-review_chromium.org Target Ref:
refs/heads/master Project:
catapult Visibility:
Public. |
Description[loading metric] filter out events during cache warming
The regexp in prepareTelemetryInternalEventPredicate did not match
the titles of events logged by cache_temperature.py.
Note for perf sheriffs: Improvements are expected in page_cycler_v2
benchmarks.
BUG=catapult:#2962
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/0d3575d73a471a53e1d1d363a349ad4536f293fc
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix in cache_temperature #
Total comments: 1
Patch Set 3 : Fix in cache_temperature (really) #Messages
Total messages: 21 (12 generated)
The CQ bit was checked by ksakamoto@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...
ksakamoto@chromium.org changed reviewers: + kouhei@chromium.org, nednguyen@google.com
https://codereview.chromium.org/2467763002/diff/1/tracing/tracing/metrics/sys... File tracing/tracing/metrics/system_health/loading_metric.html (right): https://codereview.chromium.org/2467763002/diff/1/tracing/tracing/metrics/sys... tracing/tracing/metrics/system_health/loading_metric.html:172: if (!!slice.title.match(/^telemetry\.[^.]*\.warmCache\.start$/)) Sorry about the original mistake, but can we fix the cache_temperature side instead? system_health shouldn't know about warm cache stuff. (It should just ignore telemetry internal events)
OK, PTAL https://codereview.chromium.org/2467763002/diff/1/tracing/tracing/metrics/sys... File tracing/tracing/metrics/system_health/loading_metric.html (right): https://codereview.chromium.org/2467763002/diff/1/tracing/tracing/metrics/sys... tracing/tracing/metrics/system_health/loading_metric.html:172: if (!!slice.title.match(/^telemetry\.[^.]*\.warmCache\.start$/)) On 2016/11/01 06:17:25, kouhei wrote: > Sorry about the original mistake, but can we fix the cache_temperature side > instead? > system_health shouldn't know about warm cache stuff. (It should just ignore > telemetry internal events) Done.
lgtm % below https://codereview.chromium.org/2467763002/diff/20001/tracing/tracing/metrics... File tracing/tracing/metrics/system_health/loading_metric_test.html (right): https://codereview.chromium.org/2467763002/diff/20001/tracing/tracing/metrics... tracing/tracing/metrics/system_health/loading_metric_test.html:63: title: 'telemetry.internal.warmCache.warmCache.start', Let's use something other than warmCache for the identifier
OK, I think PS3 is the right fix. PTAL
lgtm
The CQ bit was checked by ksakamoto@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 ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. BUG=catapult:#2962 ========== to ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 and loading.mobile benchmarks. BUG=catapult:#2962 ==========
Description was changed from ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 and loading.mobile benchmarks. BUG=catapult:#2962 ========== to ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 benchmarks. BUG=catapult:#2962 ==========
Description was changed from ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 benchmarks. BUG=catapult:#2962 ========== to ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 benchmarks. BUG=catapult:#2962 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm oops
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 benchmarks. BUG=catapult:#2962 ========== to ========== [loading metric] filter out events during cache warming The regexp in prepareTelemetryInternalEventPredicate did not match the titles of events logged by cache_temperature.py. Note for perf sheriffs: Improvements are expected in page_cycler_v2 benchmarks. BUG=catapult:#2962 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
