New issue
Advanced search Search tips

Issue 887204 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Make it clear the ownership and roles of blink/perf_tests/dom and blink_perf_tests/binding

Project Member Reported by hayato@chromium.org, Sep 20

Issue description

It looks this CL [1] is adding a perf-test about Custom Elements in perf_tests/dom, however, it looks perf_tests/dom is owned by the binding team [2], and most of the tests here are related to binding (for some historical reasons?).

It is unclear to me that the difference between perf_test/dom and perf_test/bindings. That confused me.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1233593
[2] https://docs.google.com/spreadsheets/d/1xaAo0_SU3iDfGdqDJZX_jRV0QtkufwHUKH3kQKF3YQs/edit#gid=0


To make it clear, how about moving some tests in perf_tests/dom to perf_tests/bindings if the tests are to measure binding performance?

Does it make sense?

dom-team is happy to have an ownership of perf_tests/dom, where there are already tests for editing, which would be better to be owned by dom/editing team.
 
Cc: jbroman@chromium.org yukishiino@chromium.org
Owner: ----
Status: Available (was: Assigned)
Jeremy has the primary ownership of bindings perf tests.  As long as Jeremy is happy, I'm happy, too.  Once Jeremy agrees, feel free to move tests.
Owner: jbroman@chromium.org
jbroman@, what do you think about this?

It looks blink developers have been putting several *dom-related* perf-tests into perf_tests/dom because the name implies that.

However, I guess the current owners of perf_tests/dom couldn't take care of these tests.
Owner: hayato@chromium.org
I have no objections to DOM team taking ownership of blink_perf.dom. I think we have it mostly for historical reasons. If you do, please also update the benchmark metadata in tools/perf/.

There might be some tests in there that are more bindings-limited than DOM-limited; if so moving them to blink_perf.bindings sounds okay to me.
Labels: -Pri-3 Pri-2
Status: Assigned (was: Available)
jbroman@,
Thanks. Let me take this.
This is not a high priority issue for us, however, I've seen some people being confused on this several times. It would be worth fixing this quickly.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21

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

commit eff49dd8acbcaaf483ae6923097f095d7eb334f4
Author: Hayato Ito <hayato@chromium.org>
Date: Fri Sep 21 14:40:35 2018

Update owners and component of blink_perf.dom

We might want to move some perf tests from perf_tests/dom to
perf_tests/bindings, which would be done in another CL later.

See  http://crbug.com/887204  for the context.

Bug:  887204 
Change-Id: Idd818d23337dfb015e3643ffc4780be764858eb1
Reviewed-on: https://chromium-review.googlesource.com/1237897
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#593175}
[modify] https://crrev.com/eff49dd8acbcaaf483ae6923097f095d7eb334f4/tools/perf/benchmark.csv
[modify] https://crrev.com/eff49dd8acbcaaf483ae6923097f095d7eb334f4/tools/perf/benchmarks/blink_perf.py

Status: Fixed (was: Assigned)

Sign in to add a comment