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

Issue 739505 link

Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 762311



Sign in to add a comment

Disable task profiler

Project Member Reported by dskiba@chromium.org, Jul 5 2017

Issue description

Task profiler is enabled by default (kInitialStartupState = ThreadData::PROFILING_ACTIVE in tracked_objects.cc), and causes ~120 KiB of allocations (see base/tracked_objects.cc group in go/fxbus for example).

I don't know how task profiler is used on Android, but disabling it will save some memory.

Design doc:
https://docs.google.com/document/d/1cibE5nAG9sdjVMfs-MkvrMtp43i9MLGqM_uNusLqiBY
 
The task profiler data is included in UMA reports.  I thought that there were some folks actively using this data for Android Chrome development, but I might be misremembering.
Components: Internals>Metrics
Interesting, I didn't know about that. The data seems to be piping to go/uma_profiler, but it looks filtering by OS is not supported?
Filtering by OS is supported via the "Version" filter... though now that I look more closely, it looks like Android is not available in the list.  So, I'd guess it's somewhat unlikely that the aggregated data is being used by folks working on Android Chrome =P
My understanding is the timing code was disabled. At some point there was a trial to enable for a small slice of users. Probably, if we're not enabling the timing code, we could also avoid the memory overhead for this.
asvitkine@ do you mean disabled on Android? Also, what is "timing code" - do you mean the thing that sends data back to us?
Yes, I do mean on Android.

Re: Timing code, it was found that collecting times of the tasks has a bigger hit on Android than other platforms. So it was disabled (I think for 99% of users). I think we still report the data (i.e. other parts that don't include run times - such as counts of task, etc), but not the timing information.

My suggestion is for the 99% of the case, we could disable everything, not just the timing information. But for the 1% of sessions where we do enable it, it makes sense to still have it all. 1% of sessions on stable should still provide useful performance data while having a much smaller impact on users.

I'll try to dig up the relevant code.
Cc: tdres...@chromium.org
Looking at server-side configs, it looks like it was 0.1% rather than 1% for stable. However, looks like the configs have expired. So we should double check the client code - it could be defaulting to actually enabling the timing info now?

+tdresser who's also been recently looking at the task profiler stuff for speed metrics
Here's the original CL that disabled it on Android:

https://codereview.chromium.org/99343002

So the part I missed in comment 9 is the piece in content_startup_flags.cc that specifically disables timing on Android by modifying command line flags.
(Wonder if we could just simplify this whole thing by using a base::Feature that's ifdef'd to DISABLED on Android and ENABLED everywhere else.)
Sorry for the delayed response, I've been OOO.

I'm looking at piggy backing on top of the task profiler to build per thread CPU usage metrics.

See https://codereview.chromium.org/2973543002/ for example.

That being said, it is much more heavyweight than I require.

Perhaps we could disable some task attribution data without disabling the profiler completely? Based on a cursory glance, it looks like attribution is responsible for the bulk of the memory consumption?
Status: Available (was: Untriaged)
Owner: brettw@chromium.org
Status: Started (was: Available)
Summary: Disable task profiler (was: Disable task profiler on Android)
I'm going to be looking at this across all platforms. I will sync up with folks on this bug next week.
Labels: -OS-Android OS-All
Just to provide some context on the bug, Brett is looking to reduce memory use and some of the task data showed up in traces. So the question is can we eliminate it if it's not useful?

We had a discussion around this and I mentioned that this data is useful on occasion, such as when it helped identify the cause of the go/m51-startup-regression-postmortem.

However, if it's causing significant memory use, I agreed that it doesn't make sense to make all our users pay for it. I suggested we can change it to be enabled just for 1% of session so that if we need to debug problems, data is still available, but most users don't have to pay the cost. We can also have a 1% control group as well and compare the data for the two to see how much the runtime data savings actually is with the new memory metrics that Erik had added.

However, Brett also mentioned there's a binary size impact of this - due to FROM_HERE macros that hold filenames as strings. This unfortunately we can't enable just for 1% users since its a compile-time thing. Brett suggested we could maybe store the PC address and symbolize it on the server, but this would require a bunch of work.

Another option is to see if we could obtain the same data from the Sampling Profiler - which records samples periodically - which both Brett and I agree would be a good solution, since it would allow us to entirely delete the C++ and JS chrome://profiler from Chrome. (Though the limitations there is we only support a subset of platforms for Sampling Profiler right now.)

Brett will write up a doc and more discussion can happen there.
If we are pulling out the task profiler, can we leave enough in place for building CPU time metrics? I don't need any of the location data for CPU time metrics, but my prototype relies on task profiler instrumentation for gathering task durations per thread.
Thinking about this more, I don't think sampling profiler data can replace what task profiler provides.

In particular, task profiler counts number of tasks posted as well as their run times - whereas the sampling profiler will just know how much time in aggregate those functions take to run. This means it's impossible to tell whether a function is actually janky (i.e. often takes over > 100ms or w/e threshold) vs. it just executes often and takes a lot of time in aggregated, but never causes jank for users (since all executions are short).

So given this, maybe the other plan that we had discussion (replacing location info with pc) might be better to pursue, else we would be losing jank data - which I think is still important for Chrome to not regress on (and we don't have any other mechanism to track this right now AFAIK).

Changing source location to PC would involve changing the protobuf format sent to UMA and updating our pipeline that consumes this - but I guess since we already do this for sampling profiler pipeline, it may not be that bad to adapt.

As for chrome://profiler - I agree that it's not very useful in its current form. Originally it was added because we didn't have data in UMA - but now we have UMA data so local aggregation doesn't as useful. What would be useful is for that page to instead show data from the last X amount of time only - let's say last 5 minutes. At least then, it could be more useful to figure out if some jank happened, what it
was. I've used the existing UI for that but it doesn't serve that use case very well because it's hard to tell if something slow was recent or old. In theory, if we just repurpose it to such a use case, the page could be greatly simplified - i.e. can remove all the JS and HTML and just have a simple text page that the code outputs - like chrome://histograms.

Cc: gab@chromium.org robliao@chromium.org
Also, +gab +robliau from Lucky Luke team who I think have used this these tools before for their thoughts.
Description: Show this description
I've used http://go/uma-profiler a long long time ago via the manual instrumentation route to find janks.

I think asvitkine@'s analysis is right with regards to the differences between the sampling profiler and the task profiler in that the sampling profiler is vulnerable to task aliasing. 

Now, having said that, there is a legitimate discussion to be had about the task profiler itself.

With the sampling profiler, it seems all first level triage of issues should go through the sampling profiler first, especially as it doesn't require manual instrumentation of code.

The task profiler should only be used if it cannot be determined from the sampling profiler if an issue exists.

This leads to a few questions (of which I don't have answers)
* From a practical standpoint, are there issues that only the task profiler can identify. In a hypothetical standpoint, the answer is yes via aliasing, but how often does this actually happen?

* Have we used the task profiler / http://go/uma-profiler recently to find and fix issues? If the answer is no and we generally agree that Chrome is operating reasonably well, perhaps we can get by without them.
Blockedon: 762311
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 6 2017

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

commit ffc24720821a71c9ea261fb810d64fa64867ca0f
Author: Brett Wilson <brettw@chromium.org>
Date: Wed Sep 06 19:11:29 2017

Remove chrome://profiler WebUI.

This removes the WebUI frontend for the task profiler. The underlying
infrastructure will be removed in a separate pass.

This saves about 30KB binary size.

BUG= 739505 

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9d871e028168104496a0ab7f68aa45e428cba278
Reviewed-on: https://chromium-review.googlesource.com/650947
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500031}
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/browser/BUILD.gn
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/browser/browser_resources.grd
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/resources/profiler/OWNERS
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/resources/profiler/profiler.html
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/resources/profiler/profiler.js
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/task_profiler/OWNERS
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/task_profiler/task_profiler_data_serializer.cc
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/task_profiler/task_profiler_data_serializer.h
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/task_profiler/task_profiler_data_serializer_unittest.cc
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/ui/webui/profiler_ui.cc
[delete] https://crrev.com/f764452748847feb84e7fd91e0075018e053779c/chrome/browser/ui/webui/profiler_ui.h
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/common/url_constants.cc
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/common/url_constants.h
[modify] https://crrev.com/ffc24720821a71c9ea261fb810d64fa64867ca0f/chrome/test/BUILD.gn

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 8 2017

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

commit b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7
Author: Brett Wilson <brettw@chromium.org>
Date: Fri Sep 08 00:47:49 2017

Remove TrackingInfo class.

Part of removing the task profiler, this removes the TrackingInfo class and the
part of the task profiler that uses it.

The task delay time from tracking info that was used by other code was moved to
PendingTask.

BUG= 739505 

Change-Id: Icc0eff6eadbd3080dda69221bea6d57d1b39af5c
Reviewed-on: https://chromium-review.googlesource.com/655637
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500459}
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/BUILD.gn
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/debug/task_annotator.cc
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/pending_task.cc
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/pending_task.h
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/threading/sequenced_worker_pool.cc
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/threading/worker_pool_posix.cc
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/threading/worker_pool_win.cc
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/tracked_objects.cc
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/tracked_objects.h
[modify] https://crrev.com/b57e3dd20dc6d0750e8c1edc165eae72b9d1cdf7/base/tracked_objects_unittest.cc
[delete] https://crrev.com/d7cdcb3e2776e69567d4457099d7af97b48c1ff6/base/tracking_info.cc
[delete] https://crrev.com/d7cdcb3e2776e69567d4457099d7af97b48c1ff6/base/tracking_info.h

Project Member

Comment 25 by bugdroid1@chromium.org, Sep 8 2017

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

commit 27e878c48028516d04f74cf7f45f6c7c8ff73a85
Author: Brett Wilson <brettw@chromium.org>
Date: Fri Sep 08 21:32:21 2017

Remove ScopedProfile and ScopedTracker.

This is part of removing the task profiler. The information that these
classes provide will no longer be used.

A number of files got the LazyInstance definition via the removed headers,
so lazy_instance includes were added in some places as needed.

Binary size improvement:
  Linux x64: 238784 bytes removed
  Android ARM: 106496 bytes removed

Bug:  739505 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I4b2c20453971df34328e6854f3ea705411527b1a
Reviewed-on: https://chromium-review.googlesource.com/655147
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500690}
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/base/BUILD.gn
[delete] https://crrev.com/8a375ca964414f5d71f15c7e8de0f37ed494bd0c/base/profiler/scoped_profile.cc
[delete] https://crrev.com/8a375ca964414f5d71f15c7e8de0f37ed494bd0c/base/profiler/scoped_profile.h
[delete] https://crrev.com/8a375ca964414f5d71f15c7e8de0f37ed494bd0c/base/profiler/scoped_tracker.cc
[delete] https://crrev.com/8a375ca964414f5d71f15c7e8de0f37ed494bd0c/base/profiler/scoped_tracker.h
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/base/win/wrapped_window_proc.h
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/cc/scheduler/scheduler.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/chrome/browser/safe_browsing/protocol_manager.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/components/domain_reliability/config.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/components/domain_reliability/monitor.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/components/machine_intelligence/ranker_model_loader.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/components/signin/core/browser/account_tracker_service.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/components/sync/driver/data_type_manager_impl.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/components/translate/core/browser/translate_script.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/content/browser/browser_main_runner.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/browser/api/usb/usb_event_router.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/browser/extension_function_dispatcher.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/browser/guest_view/app_view/app_view_guest.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/browser/guest_view/web_view/web_view_guest.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/extensions/renderer/worker_thread_dispatcher.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/google_apis/gaia/gaia_auth_fetcher.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/ipc/ipc_message_macros.h
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/cert/multi_threaded_cert_verifier.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/extras/sqlite/sqlite_persistent_cookie_store.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/http/http_network_session.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/http/http_proxy_client_socket.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/http/http_proxy_client_socket_wrapper.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/quic/chromium/quic_connection_logger.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/socket/transport_client_socket_pool.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/net/spdy/chromium/spdy_session_pool.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/ppapi/host/dispatch_host_message.h
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/storage/browser/quota/quota_manager.cc
[modify] https://crrev.com/27e878c48028516d04f74cf7f45f6c7c8ff73a85/ui/app_list/views/app_list_main_view.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Sep 9 2017

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

commit 28c7be6d655a213414d5c0d31cdb653de6b4bcb1
Author: Brett Wilson <brettw@chromium.org>
Date: Sat Sep 09 00:33:58 2017

Remove components/metrics/profiler

The task profiler is being removed. This component provides glue between
content, the task profiler in base, and the metrics service, and is not needed
any more.

Binary size improvements:
  Linux x64: 20496 bytes saved
  Android ARM: 4096 bytes saved

Bug:  739505 
Change-Id: Ica9f12dbb1787e61c4232263fcb223f1e6d47342
Reviewed-on: https://chromium-review.googlesource.com/658258
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500760}
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/android_webview/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/android_webview/browser/aw_metrics_service_client.cc
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/chrome_browser_main.h
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/metrics/chrome_metrics_service_client.h
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/metrics/first_web_contents_profiler.cc
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chromecast/browser/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chromecast/browser/metrics/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/components/metrics/BUILD.gn
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/content/DEPS
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/content/content_tracking_synchronizer_delegate.cc
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/content/content_tracking_synchronizer_delegate.h
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/ios/ios_tracking_synchronizer_delegate.cc
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/ios/ios_tracking_synchronizer_delegate.h
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/profiler_metrics_provider.cc
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/profiler_metrics_provider.h
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/profiler_metrics_provider_unittest.cc
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/tracking_synchronizer.cc
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/tracking_synchronizer.h
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/tracking_synchronizer_delegate.h
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/tracking_synchronizer_observer.cc
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/tracking_synchronizer_observer.h
[delete] https://crrev.com/ad1546384d43d720f6b31d7c510add8d094836cc/components/metrics/profiler/tracking_synchronizer_unittest.cc
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/ios/chrome/browser/ios_chrome_main_parts.h
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/ios/chrome/browser/ios_chrome_main_parts.mm
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/ios/chrome/browser/metrics/BUILD.gn
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.h
[modify] https://crrev.com/28c7be6d655a213414d5c0d31cdb653de6b4bcb1/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 11 2017

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

commit c1d46a9dbabc58425056b254c3fdd12f11d3d504
Author: Brett Wilson <brettw@chromium.org>
Date: Mon Sep 11 22:28:29 2017

Remove content task tracking.

This is the multiprocess components of the task profiling that is being
removed. This saves 27KB binary size on Linux x64, 12KB on Android ARM.

TBR=tsepez@chromium.org for security_messages.h (removal only)

Bug:  739505 
Change-Id: Ibc1858220073e94dfd34e951ed0157ca20d7c239
Reviewed-on: https://chromium-review.googlesource.com/658457
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501062}
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/browser/BUILD.gn
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/browser/browser_child_process_host_impl.cc
[delete] https://crrev.com/06b02e60ced3920a19bba3981d6de81e35c6e412/content/browser/profiler_controller_impl.cc
[delete] https://crrev.com/06b02e60ced3920a19bba3981d6de81e35c6e412/content/browser/profiler_controller_impl.h
[delete] https://crrev.com/06b02e60ced3920a19bba3981d6de81e35c6e412/content/browser/profiler_message_filter.cc
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/child/child_thread_impl.cc
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/child/child_thread_impl.h
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/common/child_process_messages.h
[modify] https://crrev.com/c1d46a9dbabc58425056b254c3fdd12f11d3d504/content/public/browser/BUILD.gn
[delete] https://crrev.com/06b02e60ced3920a19bba3981d6de81e35c6e412/content/public/browser/profiler_controller.h
[delete] https://crrev.com/06b02e60ced3920a19bba3981d6de81e35c6e412/content/public/browser/profiler_subscriber.h

Cc: brucedaw...@chromium.org chengx@chromium.org grt@chromium.org
Maybe I'm the one who uses http://go/uma-profiler the most in the past 6 months or so. So I think I should comment, no matter if we are going to retire task profiler or not. Btw, I am fairly new in Chrome (this is my 11th month), so some of my thoughts below may be wrong. I apologize in advance. A little more about me, I'm in Chrome Windows team focusing on improving Chrome performance, especially in CPU and power usage.

Quoted from comment #21 by robliao@
"Have we used the task profiler / http://go/uma-profiler recently to find and fix issues? If the answer is no and we generally agree that Chrome is operating reasonably well, perhaps we can get by without them."

1) Have we used the task profiler to find and fix issues? Yes, I have. Task profiler has helped me to fix the severe bugs in jumplist. See crbug/40407 where more than 10 related bugs are linked inside. This is a 7+ year old bug and I used 70+ CLs to fix it. See go/jumplist-icon-accum-fix and go/jumplist-boost-efficiency for the whole story if you're interested. I also used task profiler to find the severe issues in TopSites service. One example is crbug/763103. 

2) Is Chrome operating reasonably well? Yes, it does. I love Chrome.

3) Does Chrome still nasty bugs to fix? Yes, see crbug/103737, crbug/176727 for example. Similar to jumplist, there are no useful traces and we cannot repro locally. Task profiler may be the only weapon that could help according to my own experience. If you have any suggestions, that'll be great! 

Lastly, I also like the idea of removing the cost of task profiler if that's unnecessary.
Project Member

Comment 29 by bugdroid1@chromium.org, Sep 15 2017

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

commit 0241e02bedc445db6bce2e909d386ed9c1386d6b
Author: Brett Wilson <brettw@chromium.org>
Date: Fri Sep 15 22:45:35 2017

Remove tracked_objects.

This is the last step in removing the task profiler code.

A number of files were depending on this to get the LazyInstance
declaration so includes for lazy_instance are added.

BUG= 739505 

Change-Id: I5cc4dde74b40f33d274215e62fbcd72ca7df6aea
Reviewed-on: https://chromium-review.googlesource.com/665969
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502415}
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/BUILD.gn
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/base_switches.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/base_switches.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/debug/task_annotator.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/deferred_sequenced_task_runner.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/pending_task.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/run_loop.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/test/task_runner_test_template.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/platform_thread_android.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/platform_thread_fuchsia.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/platform_thread_linux.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/platform_thread_mac.mm
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/platform_thread_win.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/sequenced_worker_pool.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/sequenced_worker_pool_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/thread_perftest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/worker_pool.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/worker_pool_posix.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/worker_pool_posix.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/base/threading/worker_pool_win.cc
[delete] https://crrev.com/39160b61b45353e67f018806303e16ab74d268ca/base/tracked_objects.cc
[delete] https://crrev.com/39160b61b45353e67f018806303e16ab74d268ca/base/tracked_objects.h
[delete] https://crrev.com/39160b61b45353e67f018806303e16ab74d268ca/base/tracked_objects_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/build/sanitizers/tsan_suppressions.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/after_startup_task_utils.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/chrome_browser_field_trials_mobile.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/chrome_browser_main.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.mm
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/common/chrome_switches.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/chrome/common/chrome_switches.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/domain_reliability/util.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/domain_reliability/util_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/invalidation/impl/sync_invalidation_listener.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/metrics/metrics_service.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/pdf/renderer/pepper_pdf_host.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/search_engines/search_engine_data_type_controller_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync/driver/async_directory_type_controller_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync/driver/backend_migrator.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync/driver/backend_migrator_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync/driver/frontend_data_type_controller_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync/engine_impl/sync_encryption_handler_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync/engine_impl/sync_encryption_handler_impl_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/components/sync_wifi/wifi_credential_syncable_service_unittest.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/app/android/library_loader_hooks.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/android/content_startup_flags.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/browser_main_runner.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/dom_storage/dom_storage_session.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/dom_storage/dom_storage_task_runner.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/gpu/gpu_process_host.cc
[delete] https://crrev.com/39160b61b45353e67f018806303e16ab74d268ca/content/browser/profiler_message_filter.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/browser/utility_process_host_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/child/child_thread_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/child/child_thread_impl.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/common/child_process_messages.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/content/renderer/browser_plugin/browser_plugin.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/google_apis/gcm/engine/gcm_store_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/ios/chrome/browser/application_context_impl.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/net/url_request/url_fetcher_core.cc
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/ppapi/proxy/dispatcher.h
[modify] https://crrev.com/0241e02bedc445db6bce2e909d386ed9c1386d6b/remoting/host/native_messaging/log_message_handler.cc

Status: Fixed (was: Started)
Project Member

Comment 31 by bugdroid1@chromium.org, Sep 26 2017

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

commit 3842c830d842da7e551ebe5437e3553b7990cfbb
Author: Brett Wilson <brettw@chromium.org>
Date: Mon Sep 25 23:56:45 2017

Remove test for code profiler_event protos.

This proto will be removed in a followup (requires a server change first).

Bug:  739505 
Change-Id: I979ff8037b81e983c18f5abe1b964fba4dbb4753
Reviewed-on: https://chromium-review.googlesource.com/683243
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504221}
[modify] https://crrev.com/3842c830d842da7e551ebe5437e3553b7990cfbb/components/metrics/metrics_service_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Sep 27 2017

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

commit d61aa8912c2599020e88188d1ac28f08ee10848e
Author: Brett Wilson <brettw@chromium.org>
Date: Wed Sep 27 01:21:57 2017

Remove profiler_event proto

Now that the task profiler has been removed, the proto for sending
events is not needed either.

Bug:  739505 
Change-Id: Ic3d8b509467618427763af477d44ee7b48a573d9
Reviewed-on: https://chromium-review.googlesource.com/681576
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504542}
[modify] https://crrev.com/d61aa8912c2599020e88188d1ac28f08ee10848e/components/metrics/proto/BUILD.gn
[modify] https://crrev.com/d61aa8912c2599020e88188d1ac28f08ee10848e/components/metrics/proto/chrome_user_metrics_extension.proto
[delete] https://crrev.com/3a932e978b9d8ea67e1979290b57345d8e35e64b/components/metrics/proto/profiler_event.proto

Cc: thakis@chromium.org
 Issue 728299  has been merged into this issue.

Sign in to add a comment