Skip to content

Comments

[v10.x backport] deps: V8: cherry-pick eec10a2fd8fa#35393

Closed
Flarna wants to merge 1 commit intonodejs:v10.x-stagingfrom
dynatrace-oss-contrib:v10-backport-thenables
Closed

[v10.x backport] deps: V8: cherry-pick eec10a2fd8fa#35393
Flarna wants to merge 1 commit intonodejs:v10.x-stagingfrom
dynatrace-oss-contrib:v10-backport-thenables

Conversation

@Flarna
Copy link
Member

@Flarna Flarna commented Sep 28, 2020

Original commit message:

[promisehook] Add before/after hooks to thenable tasks

This will allow Node.js to properly track async context in thenables.

Change-Id: If441423789a78307a57ad7e645daabf551cddb57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
Reviewed-by: Camillo Bruni <[email protected]>
Reviewed-by: Sathya Gunasekaran  <[email protected]>
Commit-Queue: Gus Caplan <[email protected]>
Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Reviewed-By: Anna Henningsen [email protected]
Reviewed-By: Michaël Zasso [email protected]
Reviewed-By: Gus Caplan [email protected]
Reviewed-By: Gerhard Stöbich [email protected]
Reviewed-By: Michael Dawson [email protected]

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@Qard @Flarna
deps: V8: cherry-pick eec10a2fd8fa
795dba7 
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Sathya Gunasekaran  <[email protected]>
    Commit-Queue: Gus Caplan <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: nodejs#33778
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v10.x v8 engine Issues and PRs related to the V8 dependency. labels Sep 28, 2020
@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 28, 2020 •
edited by Flarna
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/33275/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3409

@Flarna
Copy link
Member Author

Flarna commented Sep 28, 2020

It seems there is a problem with V8 CI on v10.x-staging. My run failed but I'm quite sure it's unrelated to my changes.

I triggered V8 CI on v10.x-staging branch and it shows the same failures: see https://ci.nodejs.org/job/node-test-commit-v8-linux/3412/

  • Compile error because of a warning in mark-compact.cc for rhel7-s390x,v8test and centos7-ppcle,v8test
  • Test v8-updates/test-linux-perf fails because Couldn't find interpreted functionOne()

@nodejs/v8 Are these known issues on v10?

@gengjiawen
Copy link
Member

gengjiawen commented Sep 28, 2020

It seems there is a problem with V8 CI on v10.x-staging. My run failed but I'm quite sure it's unrelated to my changes.

I triggered V8 CI on v10.x-staging branch and it shows the same failures: see ci.nodejs.org/job/node-test-commit-v8-linux/3412

  • Compile error because of a warning in mark-compact.cc for rhel7-s390x,v8test and centos7-ppcle,v8test
  • Test v8-updates/test-linux-perf fails because Couldn't find interpreted functionOne()

@nodejs/v8 Are these known issues on v10?

cc @mmarchini Any thought on v8-updates/test-linux-perf fails ?

@Flarna
Copy link
Member Author

Flarna commented Oct 1, 2020

@nodejs/build Any recent changes in compiler versions used which could explain the unrelated warning?

@rvagg
Copy link
Member

rvagg commented Oct 2, 2020

No, shouldn't be for 10.x. But it might be a question for the IBM folks since compiler failures are on IBM platforms.

@richardlau richardlau mentioned this pull request Oct 5, 2020
Closed
@richardlau
Copy link
Member

richardlau commented Oct 5, 2020

No, shouldn't be for 10.x. But it might be a question for the IBM folks since compiler failures are on IBM platforms.

Raised #35504 to investigate.

@richardlau
Copy link
Member

richardlau commented Oct 5, 2020

Cherry-picked a commit over to v10.x-staging that ought to fix the builds for the IBM platforms.

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3443/

@Flarna Flarna requested a review from Qard October 5, 2020 19:07
@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 5, 2020
richardlau pushed a commit that referenced this pull request Oct 7, 2020
@Qard @richardlau
deps: V8: cherry-pick eec10a2fd8fa
3564424 
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Sathya Gunasekaran  <[email protected]>
    Commit-Queue: Gus Caplan <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Backport-PR-URL: #35393
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@richardlau
Copy link
Member

richardlau commented Oct 7, 2020

Landed in 3564424.

@richardlau richardlau closed this Oct 7, 2020
@Flarna Flarna deleted the v10-backport-thenables branch October 7, 2020 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Qard Qard Qard approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

+1 more reviewer

@ryzokuken ryzokuken ryzokuken approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@Flarna @nodejs-github-bot @gengjiawen @rvagg @richardlau @Qard @BethGriggs @ryzokuken