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

build: enable pointer compression #53666

Closed
wants to merge 1 commit into from
Closed

build: enable pointer compression #53666

wants to merge 1 commit into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Jul 1, 2024

I wonder why we don't enable pointer compression by default.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@anonrig anonrig requested a review from joyeecheung July 1, 2024 14:47
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Jul 1, 2024
@anonrig anonrig requested review from lemire and targos July 1, 2024 14:47
targos
targos requested changes Jul 1, 2024
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

I wonder why we don't enable pointer compression by default.

Because it puts a hard limit on the maximum heap size and there are many use cases that would be broken by it.

@joyeecheung
Copy link
Member

With pointer compression, the V8 heap is limited to 4GB and it's a hard limit that can't be raised, at least not for now. That's likely to be problematic for quite a lot of applications, so we could only make it as a special configuration, not the default one that caters to the general user base. See nodejs/TSC#790 and #26756

@anonrig
Copy link
Member Author

anonrig commented Jul 1, 2024 •
edited
Loading

Well, that's educational. Thanks @addaleax for the amazing post, @targos and @joyeecheung.

@anonrig anonrig closed this Jul 1, 2024
@joyeecheung
Copy link
Member

If anything it seems the more promising path forward is nodejs/build#3204 to provide pointer-compression builds separately for users who can work with the trade-offs. But it has been stuck for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos requested changes

@joyeecheung joyeecheung Awaiting requested review from joyeecheung

@lemire lemire Awaiting requested review from lemire

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anonrig @nodejs-github-bot @joyeecheung @targos