All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=920381

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=3d22d4f2e38ce36d37e406a2a27108b51505bdee977cf2b88ec8952943eabdaf


Bot(s) for this bug's original alert(s):

linux-perf

loading.desktop - Benchmark documentation link:
  https://bit.ly/loading-benchmarks

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/163965c5940000

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/163965c5940000

Remove empty-document special case in FrameLoader by dgozman@chromium.org
https://chromium.googlesource.com/chromium/src/+/9f050fed7e14471a28ce03090f299f1fc6c545e3
timeToFirstContentfulPaint: 42.29 → 56.87 (+14.57)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/loading-benchmarks

Is there an owner of this particular benchmark? Whom should I talk with to better understand what does it measure, so that I can figure out how this did regress?

I have cc'd loading.desktop owners. There are some info on loading metrics here: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/benchmark/harnesses/loading.md


If you look at the pinpoint link in #3, you can get access to trace from before and after your patch that can help in diagnosis as well.

Thank you for the links!

We talked with japhet@ and one guess is that we now do more stuff because NavigationScheduler::LocationChangePending returns false, in particular we may parse more html. I'll try a patch where we also check for the placeholder DL to be present. If that doesn't help, will revert for now.

Also note that this just recently improved by https://chromium-review.googlesource.com/c/chromium/src/+/1372277, which partially did the opposite thing.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/12d61adb940000

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/12d61adb940000

guid

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/125fab7b940000

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/125fab7b940000

guid

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

commit 3297f2ad2dfb0f6dca5f6adfe4fc00a2671e904f
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Tue Jan 15 03:28:35 2019

Revert "Remove empty-document special case in FrameLoader"

This reverts commit 9f050fed7e14471a28ce03090f299f1fc6c545e3.

Reason for revert: performance regression. See  crbug.com/920381 .

Original change's description:
> Remove empty-document special case in FrameLoader
> 
> After we started calling CreatePlaceholderDocumentLoader [1] for
> empty-document navigations, existing logic which checks for
> placeholder DL handles the case of empty documents as well.
> 
> [1] https://chromium-review.googlesource.com/c/chromium/src/+/1380552
> 
> Bug: none
> Change-Id: I2b1f9d73e00cd8ed3ab1f3f7f54ea930930c35dd
> Reviewed-on: https://chromium-review.googlesource.com/c/1390236
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620126}

TBR=dgozman@chromium.org,japhet@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  920381 
Change-Id: I74106eb616ece397829a338357cef7d1461a138b
Reviewed-on: https://chromium-review.googlesource.com/c/1410290
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622710}
[modify] https://crrev.com/3297f2ad2dfb0f6dca5f6adfe4fc00a2671e904f/third_party/blink/renderer/core/loader/frame_loader.cc
[modify] https://crrev.com/3297f2ad2dfb0f6dca5f6adfe4fc00a2671e904f/third_party/blink/renderer/core/loader/frame_loader.h

CL has been reverted, graphs have recovered.