search

catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.91k stars 563 forks source link

Pinpoint - Surface profiler links in UI #4132

Open simonhatch opened 6 years ago

simonhatch commented 6 years ago

Had a request recently for help getting a perf try job going with netlog profiler enabled. The data is uploaded to cloud storage, but you have to go into the task page and trawl the logs to find the links. Would be nice to just expose these in the UI. I can't find the profiler links in the resulting histogramset output, @benshayden any idea how to plumb those through?

@benshayden @dave-2

benshayden commented 6 years ago

Sorry, not sure I know what you mean by "profiler". Different from the chrome traces? I thought netlog was a chrome tracing category. Trace links are plumbed as GenericSet diagnostics named "traceUrls". Maybe "profiler" links should be plumbed as another GenericSet diagnostic?

simonhatch commented 6 years ago

Yeah different from chrome traces, here's task we kicked off: https://chromium-swarm.appspot.com/task?id=3a8e2f4535a47f10&refresh=10&show_raw=1

The commandline included --profiler=netlog, can find the files in the logs, look for:

"View generated profiler files online"

benshayden commented 6 years ago

PageTestResults handles those URLs in UploadProfilingFilesToCloud(), which is called in story_runner.RunBenchmark. Let's reserve a new GenericSet diagnostic name like "profilerUrls" and pre-allocate cloud urls for profiles in the same way that page_test_results.TelemetryInfo currently pre-allocates traceUrls so that metricMapFunction adds them as shared diagnostics. Does that sound right?

eakuefner commented 6 years ago

I think the 'profiler' architecture of Telemetry is deprecated. However, there's been ongoing work on the Telemetry side to refactor all of this file output logic into a notion of 'artifacts'.

Probably we should call the diagnostic 'artifacts', and maybe it can be a dict so that artifacts can be tagged with a key?

Side note: @nedn I thought I saw some CLs go by a while ago where someone was trying to fix the artifact handling logic in Telemetry -- do you know if anyone's actively working on that?

nedn commented 6 years ago

Yes, it's actively worked on. Artifact design: bit.ly/chromium-test-artifacts Final blocking bug: crbug.com/772216

benshayden commented 6 years ago

Probably we should call the diagnostic 'artifacts', and maybe it can be a dict so that artifacts can be tagged with a key?

There isn't a type of Diagnostic for generic dictionaries. TagMap and RelatedNameMap are for specific dictionaries. Is there any reason not to reserve separate names for GenericSets for each type of artifact?

eakuefner commented 6 years ago

I don't have a strong feeling about a specific design here. I think the right thing to do here would be to read and understand the design doc Ned linked, and then make a design decision based on that.