Skip to content
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

[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 uttampawar force-pushed the backport-22079-to-v10.x branch from eb751c8 to 8fb053a Compare October 24, 2018 22:23
@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 refack force-pushed the backport-22079-to-v10.x branch from 8fb053a to ed6b4b5 Compare October 30, 2018 17:20
@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>
@uttampawar uttampawar force-pushed the backport-22079-to-v10.x branch from ed6b4b5 to 5ed7244 Compare October 31, 2018 21:42
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>
# for free to join this conversation on GitHub. Already have an account? # 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