New issue
Advanced search Search tips

Issue 788226 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Deprecate Telemetry's iprofiler

Project Member Reported by asvitk...@chromium.org, Nov 23 2017

Issue description

Running tools/perf/run_benchmark --profile=iprofiler fails. This is on Mac 10.12.

I wanted to do this to get an instruments trace. The error is:

    self._proc = pexpect.spawn(
NameError: global name 'pexpect' is not defined

So looks like pexpect is missing.

Trying to install it shows that it doesn't work with Python2.7:

Running pexpect-4.3.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-vfljKe/pexpect-4.3.0/egg-dist-tmp-ziO0uU
  File "build/bdist.macosx-10.12-intel/egg/pexpect/_async.py", line 18
    transport, pw = yield from asyncio.get_event_loop()\

Installing Python 3.3 makes pexpect available, but then run_benchmark doesn't work:

  File "/Users/asvitkine/chromium/src/tools/perf/core/trybot_command.py", line 570
    except TrybotError, err:

SyntaxError: invalid syntax

 
Looks like pexpect is in Chromium's third_party.

So the following change to iprofiler_profiler.py gets past that error:

sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', '..'))

#try:
import pexpect  # pylint: disable=import-error
#except ImportError:
#  pass

So with the above, it does seem iprofiler gets started and gets attached to Chrome, but Chrome doesn't seem to run the original benchmark. And there's some I/O error at the end. :(
asvitkine@, these profiler are no longer supported (See https://groups.google.com/a/chromium.org/forum/#!topic/telemetry-announce/uAYGe9sLDJQ).

Is using iprofiler important for you to figure a particular perf bug?
The use case was diagnosing the cause of increasing CPU time in  crbug.com/784381 .

In the end, even with the local patch to fix this infra, I found Instruments not very useable. (I think it hung trying to symbolize Chrome.)

One thing that did end up working for me was hacking that profiler module to use the OS X "sample" utility and then running that on production Canaries (which there are no symbols for locally so the tools don't hang trying to symbolize them) and then symbolizing the reports manually using crsym/.

This worked, although the profiler runs changed the timings of the local runs too much - and there were multiple processes involved, so the info I collected wasn't very useful in the end.

I have the local patch around to both fix the iprofiler one (so it can import the file properly) and adding the 'sample' based version of that. Not sure if it's worth doing anything with them if you're planning to kill this infra anyways. Although I must say killing this doesn't give us good ability to diagnose causes of cpu regressions.
Is iprofile a stack sampling profiler? 

In general, we expect users to rely on tracing & adding extra instrumentation to debug cpu regressions.
Cc: -nednguyen@chromium.org nedngu...@google.com
iprofile is the command-line interface to Instruments (part of Xcode), which supports many different profile modes. The one that was implemented in catapult was -timeprofiler which is indeed a CPU sampling profiler.

The 'sample' command does something similar (same thing as clicking Sample in Activity Monitor) - although with the output being much more basic and not really having good tooling around it - whereas Instruments has good visualization tooling, in theory.

I'm attaching my local diff to make the stuff I was doing work, but again the value is dubious if the plan is to get rid of this support.

However, at the same time I find the suggestion of "tracing & adding extra instrumentation" to not be a great one when there's a regression in CPU use. I did try to look at traces from that problem and didn't see anything useful in them (happy to share them with someone more experienced in tracing to see if it was just me who missed something).

And instrumentation seems pretty hard for CPU use when it's for something like a page load - where there's a ton of code that runs between multiple processes - to figure out what actually is taking more cpu time now.

I think in the ideal world, we wouldn't need to use any custom platform-specific profilers and instead use something built into Chrome or catapult tools that works everywhere and can visualize CPU use in a useful way (e.g. flamegraph). e.g. similar to the visualization on uma.googleplex.com/callstacks - except there we're looking at wall clock time and not CPU time (which is useful too, but not in this case) - and that doesn't support local runs (e.g. it relies on symbolization from official builds server-side and doesn't have a high sampling frequency - which you would want if running on a single instance.)
catapult_diff.txt
3.5 KB View Download
Cc: primiano@chromium.org skyos...@chromium.org oysteine@chromium.org
Thanks for the feedbacks,  asvitkine@. I recall that we have flamegraph built in tracing in the past, but get rid of it due to the lack of usage.

Primiano/Sammi/Oystein is driving the work to overhaul tracing, so I think the best experts here about the future direction. Maybe we should just retweak this bug to making sure that tracing can support CPU analysis well?


Primiano/Sammi/Oystein: thoughts?
Yeah I personally agree with the  asvitkine's #6. We should have a generic sampling profiler that works on all platforms.
The current model where it can work only if starting some other platform specific tool via telemetry is IMHO a bit too clunky to use for the general public. Having a solution for CPU profiling is definitely one of the goals of the tracing overhauling with Perfetto (although in the next Q we first have to fix the general trace events first). 
So, at some point around Q3-4 we'll get there (at which point we might want to start some discussion with UMA folks, would be nice to share the same engine to do sampling profiling). 
In the meantime, if there isn't any active user of the current --profile=iprofiler, makes sense to me to retire it.

Thanks for the answer Primiano. It's great to know having a solution for CPU profiling is in the roadmap of perfetto.

Meanwhile, I will remove iprofiler 
Owner: nedngu...@google.com
Status: Started (was: Untriaged)
Summary: Deprecate Telemetry's iprofiler (was: tools/perf/run_benchmark --profile=iprofiler fails)
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/6b77e41141aabd9e20bb36c070ff412fec14ed92

commit 6b77e41141aabd9e20bb36c070ff412fec14ed92
Author: Nghia Nguyen <nednguyen@google.com>
Date: Tue Dec 05 14:48:11 2017

Remove Telemetry's iprofiler


Bug:  chromium:788226 
Change-Id: I8f9f256f64117355e634d3a353860473e0323e5a
Reviewed-on: https://chromium-review.googlesource.com/808744
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[delete] https://crrev.com/e0dc203974cdbdd9eb69a3afb213cfd0dcf400e7/telemetry/telemetry/internal/platform/profiler/iprofiler_profiler.py

Status: Fixed (was: Started)

Comment 13 by benhenry@google.com, Jan 16 (6 days ago)

Components: Test>Telemetry

Comment 14 by benhenry@google.com, Jan 16 (6 days ago)

Components: -Speed>Telemetry

Sign in to add a comment