Skip to content

events: improve performance caused by primordials #30577

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

antimonyGu
Copy link
Contributor

@antimonyGu antimonyGu commented Nov 21, 2019

Checklist

Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633

  • 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

@nodejs-github-bot nodejs-github-bot added the events Issues and PRs related to the events subsystem / EventEmitter. label Nov 21, 2019
lib/events.js Outdated
ownKeys: ReflectOwnKeys,
}
} = primordials;
const apply = ReflectApply;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variables ReflectApply and apply are the same thing. I think one of them can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. I have changed it. Thanks!

@nodejs-github-bot
Copy link
Collaborator

@antimonyGu
Copy link
Contributor Author

Dear all reviewers,

I see several builds fails on Jenkins, and most of them seem like a same problem. But I don't know how to fix them and the exact reason of the failure reason. Can anybody help me? Thanks very much!

@nodejs-github-bot
Copy link
Collaborator

@gireeshpunathil
Copy link
Member

@antimonyGu - there were unrelated issues that caused this failures, not a cause of worry for you!

those are being addressed, so I have re-triggered the CI. Let us see!

@antimonyGu
Copy link
Contributor Author

@gireeshpunathil Thanks very much!

@targos targos added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 22, 2019
@targos
Copy link
Member

targos commented Nov 22, 2019

Please 👍 to fast-track.

Trott pushed a commit that referenced this pull request Nov 23, 2019
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@Trott
Copy link
Member

Trott commented Nov 23, 2019

Landed in e5cbbe0.

Thanks for the contribution! 🎉

@Trott Trott closed this Nov 23, 2019
addaleax pushed a commit that referenced this pull request Nov 30, 2019
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos pushed a commit that referenced this pull request Dec 1, 2019
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@BridgeAR BridgeAR mentioned this pull request Dec 3, 2019
@BethGriggs BethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@BethGriggs BethGriggs mentioned this pull request Dec 23, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants