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

Jump to bottom

Combination of large pages and fully static does not work #23906

Closed
suresh-srinivas opened this issue Oct 26, 2018 · 1 comment
Closed

Combination of large pages and fully static does not work #23906

suresh-srinivas opened this issue Oct 26, 2018 · 1 comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.

Comments

@suresh-srinivas
Copy link
Contributor

suresh-srinivas commented Oct 26, 2018 •
edited
Loading

./configure --fully-static --use-largepages
labuser@y004:~/ssuresh/node-ssuresh$ ./node
Illegal instruction (core dumped)

I debugged this problem and when we use --fully-static, the .text segment contains libc code for mmap. This is one of the functions that is called during the actual moving of the TextRegion and one of the requirement was to not have any functions that are called in MoveTextRegionToLargePages themselves being moved.

#0  0x00000000017690c0 in mmap64 ()
#1  0x000000000060006f in node::MoveTextRegionToLargePages(node::text_region const&) ()
#2  0x000000000078519e in node::MapStaticCodeToLargePages() ()

I have a fix/workaround for it by moving all of the static code libc into the .lpstub region. Another potential fix is to use the SYSCALL interface to call the mmap, I havent tried that yet.

I will send a PR with the fix. @refack @uttampawar @gireeshpunathil @lundibundi @addaleax

  • Version: master
  • Platform: Linux
  • Subsystem: src
@lundibundi lundibundi added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 26, 2018
@bnoordhuis
Copy link
Member

Another potential fix is to use the SYSCALL interface to call the mmap

syscall() is implemented in glibc and musl as a function, not a macro, so you'd have the same issue.

You can of course use inline assembly but then you need to write that for each supported architecture.

@suresh-srinivas suresh-srinivas mentioned this issue Oct 30, 2018
Closed
4 tasks
suresh-srinivas added a commit to suresh-srinivas/node that referenced this issue Oct 30, 2018
@suresh-srinivas
src: fix fully-static & large-pages combination
f47ab8f 
Fixes: nodejs#23906
Refs: nodejs#22079

This change to ld.implicit.script moves libc static code to
.lpstub area and avoids the issue detailed in 23906

Quick performance comparision on web-tooling shows 3%
improvement for the combination over fully-static

cycles                       376,235,487,455  390,007,877,315
instructions                 700,341,146,973  714,773,201,182
itlb_misses_walk_completed        20,654,246       28,908,381
itlb_misses_walk_completed_4k     19,884,666       28,865,118
itlb_misses_walk_completed_2m_4m     769,391           43,251
Score                                   9.13             8.86
@refack refack closed this as completed in d32b5bd Nov 1, 2018
targos pushed a commit that referenced this issue Nov 2, 2018
@suresh-srinivas @targos
src: fix fully-static & large-pages combination
90872c4 
Fixes: #23906
Refs: #22079

This change to ld.implicit.script moves libc static code to
.lpstub area and avoids the issue detailed in 23906

Quick performance comparision on web-tooling shows 3%
improvement for the combination over fully-static

cycles                       376,235,487,455  390,007,877,315
instructions                 700,341,146,973  714,773,201,182
itlb_misses_walk_completed        20,654,246       28,908,381
itlb_misses_walk_completed_4k     19,884,666       28,865,118
itlb_misses_walk_completed_2m_4m     769,391           43,251
Score                                   9.13             8.86

PR-URL: #23964
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 27, 2018
@suresh-srinivas @MylesBorins
src: fix fully-static & large-pages combination
d25e94b 
Fixes: #23906
Refs: #22079

This change to ld.implicit.script moves libc static code to
.lpstub area and avoids the issue detailed in 23906

Quick performance comparision on web-tooling shows 3%
improvement for the combination over fully-static

cycles                       376,235,487,455  390,007,877,315
instructions                 700,341,146,973  714,773,201,182
itlb_misses_walk_completed        20,654,246       28,908,381
itlb_misses_walk_completed_4k     19,884,666       28,865,118
itlb_misses_walk_completed_2m_4m     769,391           43,251
Score                                   9.13             8.86

PR-URL: #23964
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
rvagg pushed a commit that referenced this issue Nov 28, 2018
@suresh-srinivas @rvagg
src: fix fully-static & large-pages combination
0417fdc 
Fixes: #23906
Refs: #22079

This change to ld.implicit.script moves libc static code to
.lpstub area and avoids the issue detailed in 23906

Quick performance comparision on web-tooling shows 3%
improvement for the combination over fully-static

cycles                       376,235,487,455  390,007,877,315
instructions                 700,341,146,973  714,773,201,182
itlb_misses_walk_completed        20,654,246       28,908,381
itlb_misses_walk_completed_4k     19,884,666       28,865,118
itlb_misses_walk_completed_2m_4m     769,391           43,251
Score                                   9.13             8.86

PR-URL: #23964
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 29, 2018
@suresh-srinivas @MylesBorins
src: fix fully-static & large-pages combination
6f249bc 
Fixes: #23906
Refs: #22079

This change to ld.implicit.script moves libc static code to
.lpstub area and avoids the issue detailed in 23906

Quick performance comparision on web-tooling shows 3%
improvement for the combination over fully-static

cycles                       376,235,487,455  390,007,877,315
instructions                 700,341,146,973  714,773,201,182
itlb_misses_walk_completed        20,654,246       28,908,381
itlb_misses_walk_completed_4k     19,884,666       28,865,118
itlb_misses_walk_completed_2m_4m     769,391           43,251
Score                                   9.13             8.86

PR-URL: #23964
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 3, 2018
@suresh-srinivas @MylesBorins
src: fix fully-static & large-pages combination
c6ad469 
Fixes: #23906
Refs: #22079

This change to ld.implicit.script moves libc static code to
.lpstub area and avoids the issue detailed in 23906

Quick performance comparision on web-tooling shows 3%
improvement for the combination over fully-static

cycles                       376,235,487,455  390,007,877,315
instructions                 700,341,146,973  714,773,201,182
itlb_misses_walk_completed        20,654,246       28,908,381
itlb_misses_walk_completed_4k     19,884,666       28,865,118
itlb_misses_walk_completed_2m_4m     769,391           43,251
Score                                   9.13             8.86

PR-URL: #23964
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

src: fix fully-static and large-pages combination suresh-srinivas/node
3 participants
@bnoordhuis @lundibundi @suresh-srinivas