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

Issue 830033 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Use the real time inside the InspectorPerformanceAgent for *_duration

Project Member Reported by jnd@chromium.org, Apr 6 2018

Issue description

To get correct metrics through Performance.getMetrics, we should use the real time inside the InspectorPerformanceAgent for *_duration and treat it as the cpu ticks.

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 4 2018

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

commit 79b28587446e62bf42c9ee569d13c1463463d3c1
Author: Johnny(Jianning) Ding <jnd@chromium.org>
Date: Fri May 04 08:32:12 2018

Using non-override time in InspectorPerformanceAgent.cpp.

Right now the InspectorPerformanceAgent returns incorrect performance metrics
(such as LayoutDuration, ScriptDuration, etc.) if there is an time override in
place. The InspectorPerformanceAgent needs to return real time elapsed.

Per offline discussion with Pavel(pfeldman@), the reason we don't expose the
non-override time API in the web platform (<blink>/platform/time.h) is that time
override is to abstract the web platform from the time. The web platform should
just deal with time and not know about whether the time is overridden. Exposing
the non-override time API in the very core part of blink will leak the override
aspect and hence compromising the whole design

BUG:  830033 
Change-Id: If3140892ac60bc853ef5f98f7b478a98ae332806
Reviewed-on: https://chromium-review.googlesource.com/988833
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Johnny Ding <jnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556012}
[add] https://crrev.com/79b28587446e62bf42c9ee569d13c1463463d3c1/third_party/WebKit/LayoutTests/inspector-protocol/emulation/performance-return-real-time-expected.txt
[add] https://crrev.com/79b28587446e62bf42c9ee569d13c1463463d3c1/third_party/WebKit/LayoutTests/inspector-protocol/emulation/performance-return-real-time.js
[modify] https://crrev.com/79b28587446e62bf42c9ee569d13c1463463d3c1/third_party/blink/renderer/core/inspector/DEPS
[modify] https://crrev.com/79b28587446e62bf42c9ee569d13c1463463d3c1/third_party/blink/renderer/core/inspector/inspector_performance_agent.cc
[modify] https://crrev.com/79b28587446e62bf42c9ee569d13c1463463d3c1/third_party/blink/renderer/core/inspector/inspector_performance_agent.h
[modify] https://crrev.com/79b28587446e62bf42c9ee569d13c1463463d3c1/third_party/blink/tools/audit_non_blink_usage.py

Project Member

Comment 2 by bugdroid1@chromium.org, May 4 2018

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

commit 425817138ff95e438278b1d359c389b44ca998d1
Author: Marc Treib <treib@chromium.org>
Date: Fri May 04 11:58:10 2018

Revert "Using non-override time in InspectorPerformanceAgent.cpp."

This reverts commit 79b28587446e62bf42c9ee569d13c1463463d3c1.

Reason for revert: performance-return-real-time.js fails on MSAN: https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20MSAN/

Original change's description:
> Using non-override time in InspectorPerformanceAgent.cpp.
> 
> Right now the InspectorPerformanceAgent returns incorrect performance metrics
> (such as LayoutDuration, ScriptDuration, etc.) if there is an time override in
> place. The InspectorPerformanceAgent needs to return real time elapsed.
> 
> Per offline discussion with Pavel(pfeldman@), the reason we don't expose the
> non-override time API in the web platform (<blink>/platform/time.h) is that time
> override is to abstract the web platform from the time. The web platform should
> just deal with time and not know about whether the time is overridden. Exposing
> the non-override time API in the very core part of blink will leak the override
> aspect and hence compromising the whole design
> 
> BUG:  830033 
> Change-Id: If3140892ac60bc853ef5f98f7b478a98ae332806
> Reviewed-on: https://chromium-review.googlesource.com/988833
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
> Commit-Queue: Johnny Ding <jnd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556012}

TBR=dgozman@chromium.org,jnd@chromium.org,haraken@chromium.org,pfeldman@chromium.org,jochen@chromium.org

Change-Id: Ic6f8feb1c77147e8b18fad12a95b1761cd13c2a0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/1044145
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556027}
[delete] https://crrev.com/0a3600c1bf36d80734992e8998447ac3edff997c/third_party/WebKit/LayoutTests/inspector-protocol/emulation/performance-return-real-time-expected.txt
[delete] https://crrev.com/0a3600c1bf36d80734992e8998447ac3edff997c/third_party/WebKit/LayoutTests/inspector-protocol/emulation/performance-return-real-time.js
[modify] https://crrev.com/425817138ff95e438278b1d359c389b44ca998d1/third_party/blink/renderer/core/inspector/DEPS
[modify] https://crrev.com/425817138ff95e438278b1d359c389b44ca998d1/third_party/blink/renderer/core/inspector/inspector_performance_agent.cc
[modify] https://crrev.com/425817138ff95e438278b1d359c389b44ca998d1/third_party/blink/renderer/core/inspector/inspector_performance_agent.h
[modify] https://crrev.com/425817138ff95e438278b1d359c389b44ca998d1/third_party/blink/tools/audit_non_blink_usage.py

Comment 3 by jnd@google.com, May 8 2018

The test script in inspector-protocol/emulation/performance-return-real-time.js is slow  on MSAN mode. It takes less 0.1 second on normal mode, but may takes 6-7 seconds on MSAn mode. Will mark it TIMEOUT in MSANExpectations.
Project Member

Comment 4 by bugdroid1@chromium.org, May 9 2018

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

commit d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2
Author: Johnny(Jianning) Ding <jnd@chromium.org>
Date: Wed May 09 19:00:44 2018

Reland "Using non-override time in InspectorPerformanceAgent.cpp."

This is a reland of 79b28587446e62bf42c9ee569d13c1463463d3c1

Original change's description:
> Using non-override time in InspectorPerformanceAgent.cpp.
> 
> Right now the InspectorPerformanceAgent returns incorrect performance metrics
> (such as LayoutDuration, ScriptDuration, etc.) if there is an time override in
> place. The InspectorPerformanceAgent needs to return real time elapsed.
> 
> Per offline discussion with Pavel(pfeldman@), the reason we don't expose the
> non-override time API in the web platform (<blink>/platform/time.h) is that time
> override is to abstract the web platform from the time. The web platform should
> just deal with time and not know about whether the time is overridden. Exposing
> the non-override time API in the very core part of blink will leak the override
> aspect and hence compromising the whole design
> 
> BUG:  830033 
> Change-Id: If3140892ac60bc853ef5f98f7b478a98ae332806
> Reviewed-on: https://chromium-review.googlesource.com/988833
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
> Commit-Queue: Johnny Ding <jnd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556012}

Change-Id: I6f04a582d63376b3d6eeb80e27615c455dba4a61
Reviewed-on: https://chromium-review.googlesource.com/1045566
Commit-Queue: Johnny Ding <jnd@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557267}
[add] https://crrev.com/d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2/third_party/WebKit/LayoutTests/inspector-protocol/emulation/performance-return-real-time-expected.txt
[add] https://crrev.com/d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2/third_party/WebKit/LayoutTests/inspector-protocol/emulation/performance-return-real-time.js
[modify] https://crrev.com/d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2/third_party/blink/renderer/core/inspector/DEPS
[modify] https://crrev.com/d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2/third_party/blink/renderer/core/inspector/inspector_performance_agent.cc
[modify] https://crrev.com/d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2/third_party/blink/renderer/core/inspector/inspector_performance_agent.h
[modify] https://crrev.com/d2ca7833e5f8992b1e8ccdfd164de0b7d37b67b2/third_party/blink/tools/audit_non_blink_usage.py

Comment 5 by jnd@chromium.org, May 30 2018

Status: Fixed (was: Assigned)

Sign in to add a comment