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.

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

[v10.x backport] src: initial large page (2M) support #23861

Closed

Conversation

uttampawar
Copy link
Contributor

PR-URL: #22079
Reviewed-By: Gireesh Punathil gpunathi@in.ibm.com
Reviewed-By: Denys Otrishko shishugi@gmail.com
Reviewed-By: Refael Ackermann refack@gmail.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. v10.x labels Oct 24, 2018
@uttampawar
Copy link
Contributor Author

uttampawar commented Oct 24, 2018

@addaleax Created this backport PR for new Large Pages Support in v10.x release. I followed all the instructions as stated in backporting-to-release-lines.md document. One step I couldn't do is to run node-test-pull-request CI job due to permission issue. Is this step different than default Travis CI job starts with any new PR changes? Any help is appreciated.

@refack
Copy link
Contributor

refack commented Oct 25, 2018

Thank you @uttampawar, here's a CI job: https://ci.nodejs.org/job/node-test-pull-request/18131/

Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

Changes to build scaffolding LGTM
RSLGTM on actual implementation.

@refack
Copy link
Contributor

refack commented Oct 25, 2018

There was a failed test with the workers test-suite, but it might be in the onderlining branch. is in the underlining branch. PR to fix is pending #23876

AssertionError [ERR_ASSERTION]: Binding contextify,Internal Binding worker,NativeModule events,NativeModule internal/async_hooks,NativeModule internal/errors,Binding uv,Binding buffer,Binding async_wrap,Internal Binding async_wrap,Binding config,Binding icu,NativeModule util,NativeModule internal/util/inspect,Binding util,NativeModule internal/util,Binding constants,Internal Binding types,NativeModule internal/util/types,NativeModule internal/validators,NativeModule internal/encoding,Internal Binding icu,NativeModule buffer,NativeModule internal/buffer,NativeModule internal/process/per_thread,NativeModule internal/process/worker_thread_only,NativeModule internal/process/stdio,NativeModule internal/worker,NativeModule assert,NativeModule internal/assert,NativeModule fs,NativeModule path,NativeModule internal/constants,Binding fs,NativeModule internal/fs/streams,NativeModule internal/fs/utils,NativeModule stream,NativeModule internal/streams/pipeline,NativeModule internal/streams/end-of-stream,NativeModule internal/streams/legacy,NativeModule _stream_readable,NativeModule internal/streams/buffer_list,NativeModule internal/streams/destroy,NativeModule internal/streams/state,NativeModule _stream_writable,NativeModule _stream_duplex,NativeModule _stream_transform,NativeModule _stream_passthrough,NativeModule internal/url,NativeModule internal/querystring,Binding url,Internal Binding messaging,Internal Binding symbols,NativeModule internal/error-serdes,NativeModule v8,Binding serdes,Binding v8,NativeModule internal/safe_globals,NativeModule url,NativeModule internal/process/warning,NativeModule internal/process/next_tick,NativeModule internal/process/promises,Internal Binding util,NativeModule internal/fixed_queue,Binding performance,Binding trace_events,NativeModule internal/inspector_async_hook,Binding inspector,Internal Binding options,NativeModule timers,Binding timer_wrap,NativeModule internal/linkedlist,NativeModule internal/timers,NativeModule internal/modules/cjs/loader,NativeModule vm,NativeModule internal/modules/cjs/helpers,NativeModule console,NativeModule internal/domexception,NativeModule worker_threads,NativeModule module
    at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/test/parallel/test-bootstrap-modules.js:14:1)

@uttampawar
Copy link
Contributor Author

@refack Do I need to do anything for this PR?

@refack
Copy link
Contributor

refack commented Oct 30, 2018

@uttampawar the issue behind #23876 was resolved so I'll try to rebase and run a new CI test.

@refack
Copy link
Contributor

refack commented Oct 30, 2018

@refack
Copy link
Contributor

refack commented Oct 30, 2018

@targos PTAL for 10.x

Copy link
Member

@lundibundi lundibundi left a comment

Choose a reason for hiding this comment

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

LGTM with a question.

[ 'OS in "freebsd linux"', {
'ldflags': [ '-Wl,-z,relro',
'-Wl,-z,now' ]
}],
Copy link
Member

Choose a reason for hiding this comment

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

Is this change intentional? If so why is it not in a separate commit (backporting this 2d4dd10)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lundibundi It was not intentional. I'll fix this and update the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lundibundi Fixed the accidental deletion of blank line.

Backport-PR-URL: nodejs#23861
PR-URL: nodejs#22079
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 4, 2018
Backport-PR-URL: #23861
PR-URL: #22079
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorins
Copy link
Contributor

landed in d1102f8

@MylesBorins MylesBorins closed this Nov 4, 2018
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Backport-PR-URL: #23861
PR-URL: #22079
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
Backport-PR-URL: #23861
PR-URL: #22079
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants