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

Issue 741662 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Participants' hotlists:
Cr8er


Sign in to add a comment

Proper tagged and released V8 commits are not shown on Chromiumdash

Project Member Reported by hablich@chromium.org, Jul 12 2017

Issue description

Note, we shouldn´t broaden the ref pattern chromiumdash looks for, e.g. to refs/* because we´d get all codereview branches that gerrit stores. We rather need to look in refs/branch-heads/* and refs/heads/*.
adolimpio@google.com will work on this.
The problem is that the program is only searching master, not refs/heads. It only uses the refs/ when pointing to a specific branch. At the moment this only seems to happen when searching for milestones. So handling multiple branches and refs should be implemented.
One way of doing it would be querying for heads and branch-heads to get a list of branch names, and then query commits from each branch.
Cc: prasadv@google.com amin...@google.com
Cc: -amin...@google.com -prasadv@google.com amineer@chromium.org pras...@chromium.org
This is probably due to my unfamiliarity with V8 processes, but can someone help me understand the motivation here?  Does the V8 team use each and every branch / version that is cut, and if so in what capacity?

I'm asking because (1) the details in c#0 are not entirely accurate (more on this below) and (2) we are throwing data out for Chromium as well in the same way some V8 data is not presented due to tradeoffs in processing cost / usage benefit, so I'd be curious to know if there is add'l value we're missing in not processing all V8 branches.

Re: (1) being inaccurate, the first commit (https://chromium.googlesource.com/v8/v8/+/175a595935ba451d675245238d3c2f9ba7075cbc) actually is shown, the constructed link is wrong - see here instead: https://chromiumdash.appspot.com/commit/175a595935ba451d675245238d3c2f9ba7075cbc.  For Android (the default) it says not deployed, but that's a separate bug that should fixed in the new version, toggle platform to Windows and you'll see it appear correctly.  Additionally, for the third commit referenced in c#0 (https://chromium.googlesource.com/v8/v8/+/a8a39548a410205fe49dbd5daca8cc601ade2d16) is not listed in Chromium Dash, but the root of the CP (0a67fdf63aeddbcd8ec955fa3fa3c3e56f8e15ce) is present as expected: https://chromiumdash.appspot.com/commit/7dcd046699dbd2699cf12ec7e2c6c99a8701c041

Re: (2), Chromium branches every single day as well, but we're also not polling those branches *until they ship* - at which point they're polled.  Yes, this means we don't have all data, but it's far cheaper to only monitor a few branches and then evaluate the one-off branches on demand when they're actually released.  Does V8 use versions that aren't associated with a Chromium release branch (like "6.1.201") outside of the Chromium release process (e.g. with Node or in other places)?  If not, what's the intended use?

==========

So, with all of that said - I'm not against doing this, just want to understand context / motivation a bit better.  In fact, on further reflection, one of the things I would like to do is be able to inform users in which Chromium branch their change first landed (as that is sometimes useful and different than the first release it shipped in - maybe that's your value-add as well?) - and it seems like the solution you propose might do exactly that, so perhaps we should implement this across both Chromium and V8 rather than just in V8 alone.
My motivation wasn't shipping status, but merely reflecting the state "as is" of the Git repo. In particular we'd wanna see all the cherry-pick commits as well. The branches alone don't give much value.

Are your concerns about processing costs justified? It should be rather cheap to query all refs in heads and branch-heads.

Sorry for the bad link, but I don't quite get why the commit from 1) is there, but the commit from 3) isn't. Even though both have rolled into Chromium.

Was maybe 1) picked by the CommitVersionPollerWorker because it was in a shipped version, while 3) never ended up in a Canary and, hence, didn't get processed by the CommitVersionPollerWorker?
Here's a compromise idea. We could add an extra task queue for Chromium DEPS changes on Chromium master. Each time we add a Chromium master commit that changes the DEPS file we could:
- Insert a task that analyzes the DEPS change, e.g. get before/after V8/Skia/Webrtc/etc revision
- Insert a task that adds this revision range, just like we do for shipped versions

We might need something like this later anyways for establishing the relation between Rolls and depsed commits.
Cc: hablich@chromium.org
Owner: adolimpio@google.com
That's an interesting approach (in c#8) and I think that can be explored - that said I'm not against the approach suggested above of polling branches either, I don't think the overhead would be that extreme - just trying to balance use cases of the data vs complexity of fetching and storing all of it.  IMO you're OK to try to experiment with either approach, but let's ensure Prasad concurs.
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/8df0b6f7c30a8a65842989d61ee4b67a3dd86945

commit 8df0b6f7c30a8a65842989d61ee4b67a3dd86945
Author: Andrea D'Olimpio <adolimpio@google.com>
Date: Tue Jul 18 12:37:24 2017

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/256eb9160546724d507008aa56cb6003343b1877

commit 256eb9160546724d507008aa56cb6003343b1877
Author: Andrea D'Olimpio <adolimpio@google.com>
Date: Tue Jul 18 18:03:04 2017

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 24 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/99d308024ecbfcf32114f3ae8b093cd08fcd2f76

commit 99d308024ecbfcf32114f3ae8b093cd08fcd2f76
Author: Andrea D'Olimpio <adolimpio@google.com>
Date: Mon Jul 24 16:47:04 2017

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 24 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/6220def794021cecda398ea4527e985c58e2bef2

commit 6220def794021cecda398ea4527e985c58e2bef2
Author: Andrea D'Olimpio <adolimpio@google.com>
Date: Mon Jul 24 16:52:33 2017

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 25 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/0d91b77097b648582f6a7be8a9ee7696407752a1

commit 0d91b77097b648582f6a7be8a9ee7696407752a1
Author: Andrea D'Olimpio <adolimpio@google.com>
Date: Tue Jul 25 07:43:54 2017

there's a design doc explaining the approach taken to solve this bug.
https://docs.google.com/a/google.com/document/d/1Qw8aP4cmM-oN0oAGzPdvYDQhJxBAIsHiPejBAex7HIE/edit?usp=sharing
Project Member

Comment 17 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/1f36a370e10b77d27957f959644898ce19b2c229

commit 1f36a370e10b77d27957f959644898ce19b2c229
Author: Andrea D'Olimpio <adolimpio@google.com>
Date: Thu Jul 27 09:56:27 2017

Status: Fixed (was: Assigned)
Labels: Proj-V8-Cr8er

Sign in to add a comment