New issue
Advanced search Search tips

Issue 860246 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 860247



Sign in to add a comment

Benchmark Queueing Time Estimator

Project Member Reported by tdres...@chromium.org, Jul 4

Issue description

The sampling profiler shows EQT calculation being pretty expensive.

We should add a microbenchmark, and see if we can speed things up a bit.
 
Blocking: 860247
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 6

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

commit 699ed793ea8cdf710127bcbfd7e4cbfcd0366748
Author: Nicolas Pena <npm@chromium.org>
Date: Fri Jul 06 17:53:53 2018

Add QueueingTimeEstimatorPerfTest

This CL adds a microbenchmark for QueueingTimeEstimator that runs lots
of tasks and computes ExpectedQueueingTime on them. The test is DISABLED
as it takes 6 seconds to run and will only be used manually to compute
improvements.

Bug:  860246 
Change-Id: I50d5471ed4628bf144b3da35b66d597ce6eac3c3
Reviewed-on: https://chromium-review.googlesource.com/1126444
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573012}
[modify] https://crrev.com/699ed793ea8cdf710127bcbfd7e4cbfcd0366748/third_party/blink/renderer/platform/scheduler/BUILD.gn
[modify] https://crrev.com/699ed793ea8cdf710127bcbfd7e4cbfcd0366748/third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator_unittest.cc
[add] https://crrev.com/699ed793ea8cdf710127bcbfd7e4cbfcd0366748/third_party/blink/renderer/platform/scheduler/test/DEPS
[add] https://crrev.com/699ed793ea8cdf710127bcbfd7e4cbfcd0366748/third_party/blink/renderer/platform/scheduler/test/queueing_time_estimator_perf_test.cc
[add] https://crrev.com/699ed793ea8cdf710127bcbfd7e4cbfcd0366748/third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.cc
[add] https://crrev.com/699ed793ea8cdf710127bcbfd7e4cbfcd0366748/third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 6

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

commit aa0f21ff760b8f8804d138086bd9ab8803462ddb
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Jul 06 18:18:18 2018

Revert "Add QueueingTimeEstimatorPerfTest"

This reverts commit 699ed793ea8cdf710127bcbfd7e4cbfcd0366748.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 573012 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzY5OWVkNzkzZWE4Y2RmNzEwMTI3YmNiZmQ3ZTRjYmZjZDAzNjY3NDgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/Linux%20x64/68468

Sample Failed Step: compile

Original change's description:
> Add QueueingTimeEstimatorPerfTest
> 
> This CL adds a microbenchmark for QueueingTimeEstimator that runs lots
> of tasks and computes ExpectedQueueingTime on them. The test is DISABLED
> as it takes 6 seconds to run and will only be used manually to compute
> improvements.
> 
> Bug:  860246 
> Change-Id: I50d5471ed4628bf144b3da35b66d597ce6eac3c3
> Reviewed-on: https://chromium-review.googlesource.com/1126444
> Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#573012}

Change-Id: Id16f49bb141fcc9fdb5e76bb606e8b7171a76c41
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  860246 
Reviewed-on: https://chromium-review.googlesource.com/1128141
Cr-Commit-Position: refs/heads/master@{#573017}
[modify] https://crrev.com/aa0f21ff760b8f8804d138086bd9ab8803462ddb/third_party/blink/renderer/platform/scheduler/BUILD.gn
[modify] https://crrev.com/aa0f21ff760b8f8804d138086bd9ab8803462ddb/third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator_unittest.cc
[delete] https://crrev.com/9d662169bf9c7f34df828294c9e0f65d56d3072b/third_party/blink/renderer/platform/scheduler/test/DEPS
[delete] https://crrev.com/9d662169bf9c7f34df828294c9e0f65d56d3072b/third_party/blink/renderer/platform/scheduler/test/queueing_time_estimator_perf_test.cc
[delete] https://crrev.com/9d662169bf9c7f34df828294c9e0f65d56d3072b/third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.cc
[delete] https://crrev.com/9d662169bf9c7f34df828294c9e0f65d56d3072b/third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 11

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

commit dc2ddab54de08306b065f602a453f903a9df0a29
Author: Nicolas Pena <npm@chromium.org>
Date: Wed Jul 11 17:04:44 2018

Reland "Add QueueingTimeEstimatorPerfTest"

This is a reland of 699ed793ea8cdf710127bcbfd7e4cbfcd0366748

Original change's description:
> Add QueueingTimeEstimatorPerfTest
> 
> This CL adds a microbenchmark for QueueingTimeEstimator that runs lots
> of tasks and computes ExpectedQueueingTime on them. The test is DISABLED
> as it takes 6 seconds to run and will only be used manually to compute
> improvements.
> 
> Bug:  860246 
> Change-Id: I50d5471ed4628bf144b3da35b66d597ce6eac3c3
> Reviewed-on: https://chromium-review.googlesource.com/1126444
> Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#573012}

Bug:  860246 
Change-Id: I9828081556326ae76f81b48ef65ad29812e65ad2
Reviewed-on: https://chromium-review.googlesource.com/1127962
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574201}
[modify] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/scheduler/BUILD.gn
[modify] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/scheduler/test/DEPS
[add] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/scheduler/test/queueing_time_estimator_perf_test.cc
[add] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.cc
[add] https://crrev.com/dc2ddab54de08306b065f602a453f903a9df0a29/third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.h

Cc: tdres...@chromium.org
 Issue 832883  has been merged into this issue.
Is this fixed?
Owner: npm@chromium.org
Status: Fixed (was: Available)
I'm going to say yes

Sign in to add a comment