New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 635449 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

find_unmapped_histograms.py has many false positives

Project Member Reported by ricea@chromium.org, Aug 8 2016

Issue description

The tools/metrics/histograms/find_unmapped_histograms.py script finds histograms which are defined in the source but missing from histograms.xml. Unfortunately it produces many false positives.

The main reason for false positives appears to be files which define their own UMA_HISTOGRAM_FOO macros which treat the first parameter differently.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4cb8812fb938cec4dad22432e83fac155227089

commit e4cb8812fb938cec4dad22432e83fac155227089
Author: ricea <ricea@chromium.org>
Date: Mon Aug 15 05:45:33 2016

find_unmapped_histograms.py: filter filenames

Reduce false positives from tools/metrics/find_unmapped_histograms.py by
skipping file names that are not C++ or Objective C++, and also skipping
filenames for tests.

Diff before / after this change:

***************
*** 23,26 ****
  WARNING: chrome/browser/browser_shutdown.cc contains non-literal histogram name <time_per.c_str(>
- WARNING: tools/metrics/histograms/find_unmapped_histograms.py contains non-literal histogram name <'
-   locations = RunGit(['gs'>
  WARNING: cc/raster/gpu_raster_buffer_provider.cc contains non-literal histogram name <base::StringPrintf("Renderer4.%s.PartialRasterPercentageSaved.Gpu">
--- 23,24 ----
***************
*** 60,62 ****
  INFO:   BeginResult - 0x6f42084be75ea43b
- INFO:   Bla.Foo.Dummy - 0x0c89f87e6b2dcf94
  INFO:   Bookmarks.CreateBookmarkIndexTime - 0x22dbb7a64293f927
--- 58,59 ----
***************
*** 156,158 ****
  INFO:   Flash.UsesGPU - 0x19301ada6a59b0e9
- INFO:   FooGroup.FooName - 0xbabc9e98ceaccbdf
  INFO:   GCM.DataMessageReceived - 0x6ac32c312317b760
--- 153,154 ----
***************
*** 265,270 ****
  INFO:   Shutdown.renderers.total - 0xaf283dea3598b176
- INFO:   Snafu_Dummy - 0x9d01478e32b83f00
  INFO:   SoftwareReporter.Cleaner.ExitCode - 0x1f78f7cc99e41d01
  INFO:   SoftwareReporter.ExitCode - 0x943ddee9c3ecc370
- INFO:   Sparse - 0x7407fb7e6a4df639
  INFO:   SpellCheck.CheckedWords - 0xb64739e0c80469f2
--- 261,264 ----
***************
*** 296,301 ****
  INFO:   TabRestore.read_session_file_time - 0x95a3ff3d6e153b3a
- INFO:   TestLongTimer0 - 0x85050dce02aa6dad
- INFO:   TestLongTimer1 - 0x5c0a19ba0b8828a1
- INFO:   TestTimer0 - 0xa180e82fa5050e6a
- INFO:   TestTimer1 - 0x95e83004778c4340
  INFO:   TextInputClient.CharacterIndex - 0x518547f3e632e446
--- 290,291 ----
***************
*** 307,311 ****
  INFO:   Touchpad.ThreeFingerClick.Started - 0x708f17ee567d8f4e
- INFO:   UmaHistogram - 0x46fb9873be49b3d1
- INFO:   UmaHistogramManager - 0x931b3d1b039fcfcb
- INFO:   UmaHistogramManager2 - 0xf6ec768ea672eae9
  INFO:   Unknown - 0x88183b946cc5f0e8
--- 297,298 ----
***************
*** 317,319 ****
  INFO:   WebP.DecodedImageFormat - 0xb8cc017ee20467b6
- INFO:   test - 0x098f6bcd4621d373
  INFO:   websql.Async.VacuumResult - 0x0abe7435cacb1f70

R=isherman@chromium.org
BUG= 635449 

Review-Url: https://codereview.chromium.org/2223063004
Cr-Commit-Position: refs/heads/master@{#411940}

[modify] https://crrev.com/e4cb8812fb938cec4dad22432e83fac155227089/tools/metrics/histograms/find_unmapped_histograms.py

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c65a4b5cb7790a84ed716d5c7ad01a1906e34c97

commit c65a4b5cb7790a84ed716d5c7ad01a1906e34c97
Author: ricea <ricea@chromium.org>
Date: Mon Aug 15 11:48:49 2016

find_unmapped_histograms.py: Skip unknown histogram names

Previously, tools/metrics/find_unmapped_histograms.py would attempt to
process all macros which contained the string UMA_HISTOGRAM. Some of
these had different semantics and so produced false positives.

Instead, add a whitelist of macros that the script knows how to
process. Warnings are issued about other macros which are encountered.

Diff between before / after output:

 WARNING: base/android/animation_frame_time_histogram.cc contains non-literal histogram name <histogram_name.c_str(>
+WARNING: chromecast/base/metrics/cast_histograms.h:25: Unknown macro name: <UMA_HISTOGRAM_CUSTOM_TIMES_NO_CACHE>
+WARNING: chromecast/base/metrics/cast_histograms.h:31: Unknown macro name: <UMA_HISTOGRAM_CUSTOM_COUNTS_NO_CACHE>
+WARNING: chromecast/base/metrics/cast_histograms.h:37: Unknown macro name: <UMA_HISTOGRAM_ENUMERATION_NO_CACHE>
 WARNING: content/renderer/pepper/content_decryptor_delegate.cc contains non-literal histogram name <"Media.EME." + media::GetKeySystemNameForUMA(key_system>
@@ -14,2 +17,3 @@
 WARNING: chrome/browser/prerender/prerender_histograms.cc contains non-literal histogram name <base::StringPrintf("Prerender.OmniboxNavigationsUsedPrerenderCount%s">
+WARNING: cc/trees/layer_tree_host_impl.cc:164: Unknown macro name: <DEFINE_SCOPED_UMA_HISTOGRAM_TIMER>
 WARNING: cc/trees/layer_tree_host_impl.cc contains non-literal histogram name <base::StringPrintf("Compositing.%s.PictureMemoryUsageKb">
@@ -19,12 +23,20 @@
 WARNING: chrome/browser/download/download_danger_prompt.cc contains non-literal histogram name <base::StringPrintf("%s.%s.Proceed">
+WARNING: components/startup_metric_utils/browser/startup_metric_utils.cc:128: Unknown macro name: <UMA_HISTOGRAM_WITH_TEMPERATURE>
+WARNING: components/startup_metric_utils/browser/startup_metric_utils.cc:160: Unknown macro name: <UMA_HISTOGRAM_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT>
+WARNING: components/startup_metric_utils/browser/startup_metric_utils.cc:179: Unknown macro name: <UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE>
+WARNING: components/startup_metric_utils/browser/startup_metric_utils.cc:191: Unknown macro name: <UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT>
+WARNING: base/metrics/histogram_macros.h:275: Unknown macro name: <SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER>
+WARNING: base/metrics/histogram_macros.h:285: Unknown macro name: <SCOPED_UMA_HISTOGRAM_TIMER_UNIQUE>
 WARNING: components/data_reduction_proxy/core/browser/data_reduction_proxy_compression_stats.cc contains non-literal histogram name <uma>
 WARNING: cc/raster/one_copy_raster_buffer_provider.cc contains non-literal histogram name <base::StringPrintf("Renderer4.%s.PartialRasterPercentageSaved.OneCopy">
+WARNING: cc/scheduler/compositor_timing_history.cc:108: Unknown macro name: <UMA_HISTOGRAM_CUSTOM_TIMES_VSYNC_ALIGNED>
+WARNING: cc/scheduler/compositor_timing_history.cc:117: Unknown macro name: <UMA_HISTOGRAM_CUSTOM_TIMES_DURATION>
 WARNING: chrome/browser/browser_shutdown.cc contains non-literal histogram name <time.c_str(>
 WARNING: chrome/browser/browser_shutdown.cc contains non-literal histogram name <time_per.c_str(>
+WARNING: content/child/web_database_observer_impl.cc:48: Unknown macro name: <UMA_HISTOGRAM_WEBSQL_RESULT>
+WARNING: cc/tiles/tile_manager.cc:40: Unknown macro name: <DEFINE_SCOPED_UMA_HISTOGRAM_AREA_TIMER>
 WARNING: cc/raster/gpu_raster_buffer_provider.cc contains non-literal histogram name <base::StringPrintf("Renderer4.%s.PartialRasterPercentageSaved.Gpu">
-WARNING: chromecast/browser/metrics/external_metrics.cc contains non-literal histogram name <sample.name(>
-WARNING: chromecast/browser/metrics/external_metrics.cc contains non-literal histogram name <sample.name(>
+WARNING: chrome/browser/net/request_source_bandwidth_histograms.cc:41: Unknown macro name: <UMA_HISTOGRAM_RESPONSE_KB>
 WARNING: chrome/browser/extensions/api/metrics_private/metrics_private_api.cc contains non-literal histogram name <params->metric_name>
 WARNING: chrome/browser/predictors/autocomplete_action_predictor.cc contains non-literal histogram name <base::StringPrintf("Prerender.OmniboxNavigationsCouldPrerender%s">
-WARNING: cc/base/histograms.h contains non-literal histogram name <//       ScopedReticulateSplinesTimer>
 WARNING: cc/base/histograms.h contains non-literal histogram name <base::StringPrintf(time_histogram>
@@ -57,6 +69,4 @@
 INFO:   Autofill.KeyboardAccessoryButtonsIOS_ScreenReaderOn - 0x08f97fb1a13af290
-INFO:   BeginResult - 0x6f42084be75ea43b
 INFO:   Bookmarks.CreateBookmarkIndexTime - 0x22dbb7a64293f927
 INFO:   Bookmarks.DecodeTime - 0x807c2068685e48bb
-INFO:   Browser - 0xef15fd2f45e6bb5c
 INFO:   Cache.ActiveCapacityMB - 0x9a62dfb292eb2144
@@ -75,3 +85,2 @@
 INFO:   Cellular.SIMLocked - 0x7fb31c2b69aa80fb
-INFO:   ChangeVersionResult - 0xfa5be9d5d447cd80
 INFO:   ChildProcessSecurityPolicy.FilePermissionPathLength - 0x92c6db4cf6e580c5
@@ -87,3 +96,2 @@
 INFO:   CloudPrint.XmppTimeout - 0xb948540eef7eacfa
-INFO:   CommitResult - 0xc0eecbfb87118d7a
 INFO:   ConflictingModule.UserSelection - 0x8f6918337f598cd5
@@ -228,3 +236,2 @@
 INFO:   Omnibox.SearchProvider.GetMostRecentKeywordTermsDefaultProviderTime - 0x75e7670ee11aee48
-INFO:   OpenResult - 0x8d097f7509702085
 INFO:   Overscroll.ScreenshotSize - 0xed29f0168cc9005e
@@ -250,3 +257,2 @@
 INFO:   RemovableDeviceNotificationsLinux.device_partition_size_available - 0xb8a150e488fefab1
-INFO:   Renderer - 0x07b20ae970048fc2
 INFO:   ResourceBundle.LoadLocaleResourcesError - 0x88915b4c5ad3bda3
@@ -280,3 +286,2 @@
 INFO:   SpellCheck.api.showUI - 0xd0c30938b7a87497
-INFO:   StatementResult - 0x345b32983c99be05
 INFO:   Storage.BlobItemSize.CacheEntry - 0xf20160aaa3d807b5
@@ -297,3 +302,2 @@
 INFO:   Touchpad.ThreeFingerClick.Started - 0x708f17ee567d8f4e
-INFO:   Unknown - 0x88183b946cc5f0e8
 INFO:   VirtualKeyboard.FirstLoadTime - 0xf2437a04b25a860e

R=isherman@chromium.org
BUG= 635449 

Review-Url: https://codereview.chromium.org/2223293004
Cr-Commit-Position: refs/heads/master@{#411955}

[modify] https://crrev.com/c65a4b5cb7790a84ed716d5c7ad01a1906e34c97/tools/metrics/histograms/find_unmapped_histograms.py

Comment 3 by ricea@chromium.org, Aug 26 2016

Status: Fixed (was: Started)

Sign in to add a comment