+Tim: this probably would remove the need to test UMA's FMP formula is the same as trace-based's formula.

I think this is a great idea.

Have we verified that the two implementations return precisely the same results? We should be able to gather the data from both implementations on the same page load, and spot check a few cases.

If there are differences, we should make sure to understand them.

ksakamoto@: what was the name of trace event that you put in for FMP uma? We can easily grab the traces produced by pcv2 benchmarks from the perf waterfall to spot check.

Deleted: 7iBJCHxg9SD.png 273 KB

	7iBJCHxg9SD.png 273 KB View Download

There are two differences between the two implementations:

1. They log slightly different timings in a paint operation.

The trace implementation uses the start time of devtools.timeline.Paint event.
The UMA implementation logs a timestamp at the end of GraphicsLayer::paintWithoutCommit, which is close to the end time of devtools.timeline.Paint. Typically, the difference is < 10ms.

I think we should use the latter in the trace implementation too, for consistency with trace-based TTFCP which already uses the latter.

2. Layout observation period of UMA implementation

The trace implementation scans through all the layout events until the end of trace file, to find the most significant layout.
The UMA implementation observes layout operations until network stable (2 seconds of no network activity), and reports the timestamp of paint after the most significant layout within this period.

I think we should keep current behavior in trace-based implementation, because:

- It would be inconvenient if one has to wait until fully loaded + 2 secs to get TTFMP in chrome://tracing. Also, some pages never reach network stable, e.g. pages that use long-polling.
- We may change the UMA implementation's layout observation strategy in the future. Early UMA data from canary indicates that FMP is not logged for ~30% of page loads because user left page before network stable. Maybe we should use shorter observation period.


Implementation plan:

- In Blink, log a trace event every time m_provisionalFirstMeaningfulPaint [1] is updated.
- TBMv2 finds the last trace event (per frame), and report its timestamp as FMP.
- Collect traces for Alexa top 500 sites, compare FMP from both implementations.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp?sq=package:chromium&l=70

Re #3:

Thanks for the tip.
That is blink.user_timing.FirstMeaningfulPaint trace event, but it's not logged until network stable (2 seconds of no network activity). Currently PCv2 doesn't wait for that (it will soon - kouhei is adding 5s wait for TTI).

Thanks for the details. This sounds great to me.

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

commit 1b6b36c1f28b31bdcf35d207d83e2693b75b5408
Author: ksakamoto <ksakamoto@chromium.org>
Date: Thu Aug 25 03:44:36 2016

Add trace event for FirstMeaningfulPaint candidates

This adds a trace event that is logged every time the provisional
FirstMeaningfulPaint timestamp is updated.

Trace-based implementations can find FirstMeaningfulPaint for the page
load by looking for the last event in the trace.

BUG= 638124 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/1b6b36c1f28b31bdcf35d207d83e2693b75b5408/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp

I've run the two FMP implementation on the Alexa top sites and compared the results.

Report: https://docs.google.com/document/d/16ItLkzm0IX2RKoAlztqrAmv30L9mJcDeYLJuGsH6QUo/edit?usp=sharing

I think now we understand the differences, so we can go ahead and update the TBMv2 implementation to use Blink-side implementation.

Thanks, updating the TBMv2 implementation to use Blink's implementation SGTM.

Thanks for the detective work to explain the differences. It's exciting to know that switching TBMv2 implementation will make the metrics better capture's human perceived fmp.

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

commit e7bab67cfdc033f59e2742357779ad34fe3182c7
Author: nednguyen <nednguyen@google.com>
Date: Wed Aug 31 14:11:40 2016

Manually roll src/third_party/catapult/ 578e20909..fa2c4f219 (13 commits).

This also update test_runner.pydeps to include changes to references to
catapult_base directory.

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/578e20909005..fa2c4f21907f

$ git log 578e20909..fa2c4f219 --date=short --no-merges --format='%ad %ae %s'
2016-08-30 benjhayden Rename Histogram.add() to addSample().
2016-08-30 benjhayden Replace ScalarNumerics with Histograms in value_set_test and value_set_table_test.
2016-08-30 ksakamoto Update FirstMeaningfulPaint to use Blink's implementation
2016-08-30 aiolos Revert of [polymer] - Sort series data before adding to legend. (patchset #1 id:1 of https://codereview.chromium.org/2233763003/ )
2016-08-30 benjhayden Refactor NumericBuilder to HistogramBinBoundaries.
2016-08-30 fmeawad v8CallStats: Add a map between URL and known domain names
2016-08-30 eakuefner [Catapult] Update .gitignore to point to new locations in common/
2016-08-30 washingtonp Pass in custom options to Systrace agents
2016-08-30 nednguyen [telemetry] Only disable trace profiler for TBM
2016-08-30 jbudorick [devil] Re-add the reset_usb import to unblock the catapult roll.
2016-08-30 fmeawad v8CallStats: Update the arg name
2016-08-30 benjhayden Migrate memoryMetric from ScalarNumerics to Histograms.
2016-08-30 nednguyen Remove catapult_base/ and move its files to common/py_utils/

BUG= 638124 ,  642716 

TBR=catapult-sheriff@chromium.org

patch from issue 2298183003 at patchset 1 (http://crrev.com/2298183003#ps1)

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

[modify] https://crrev.com/e7bab67cfdc033f59e2742357779ad34fe3182c7/DEPS
[modify] https://crrev.com/e7bab67cfdc033f59e2742357779ad34fe3182c7/build/android/test_runner.pydeps

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

commit b2bd8854bbbee8ccab426091a29369564710bd1a
Author: ksakamoto <ksakamoto@chromium.org>
Date: Thu Sep 01 23:24:41 2016

[PCv2] Remove tracing categories only needed for old TTFMP implementation

** PERF SHERRIFS: this will cause regression to TimeToFirstMeaningfulPaint metric but it's expected **

BUG= 638124 

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

[modify] https://crrev.com/b2bd8854bbbee8ccab426091a29369564710bd1a/tools/perf/benchmarks/page_cycler_v2.py