Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

lib: remove outdated optimizations #27380

Closed
wants to merge 1 commit into from
Closed

lib: remove outdated optimizations #27380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions lib/internal/process/task_queues.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ function processTicksAndRejections() {

class TickObject {
constructor(callback, args, triggerAsyncId) {
// This must be set to null first to avoid function tracking
// on the hidden class, revisit in V8 versions after 6.2
Copy link
Member

@joyeecheung joyeecheung Apr 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Are these related to https://bugs.chromium.org/p/v8/issues/detail?id=5456 ? @apapirovski
AFAIK constant field tracking is not yet shipped in V8 but the issue is closed so I assume the deopt storm issue has been alleviated by other patches in the upstream.

Copy link
Member

Choose a reason for hiding this comment

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

Yes but last I tried this, it was still an issue. We should create a benchmark that actually tests this if we want to be certain. (i recently tried this with timers and still saw perf penalty.)

this.callback = null;
this.callback = callback;
this.args = args;

Expand Down
3 changes: 0 additions & 3 deletions lib/timers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,6 @@ const Immediate = class Immediate {
constructor(callback, args) {
this._idleNext = null;
this._idlePrev = null;
// This must be set to null first to avoid function tracking
// on the hidden class, revisit in V8 versions after 6.2
this._onImmediate = null;
this._onImmediate = callback;
this._argv = args;
this._destroyed = false;
Expand Down