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

The property heap_size_limit from getHeapStatistics is incorrect for memory over 4032MB #10185

Closed
rdominy opened this issue Dec 8, 2016 · 1 comment
Labels
process Issues and PRs related to the process subsystem. v8 engine Issues and PRs related to the V8 dependency.

Comments

@rdominy
Copy link

rdominy commented Dec 8, 2016

  • Version: 7.2.1
  • Platform: Linux 3.10.0-327.10.1.el7.x86_64 deps: update openssl to 1.0.1j #1 SMP Tue Feb 16 17:03:50 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: v8

When calling the v8 module's getHeapStatistics function the heap_size_limit property is incorrect for memory sizes set over 4GB. Specifically if you set max_old_space >= 4032, heap_size_limit is incorrect. To reproduce:

node --max_old_space_size=3072

require('v8').getHeapStatistics().heap_size_limit
3288334336

This is as you would expect, but look at what 4096 does:

node --max_old_space_size=4096

require('v8').getHeapStatistics().heap_size_limit
67108864

64 MB???? That's not right. After some trial and error, the magic breaking point is 4032:

node --max_old_space_size=4032

require('v8').getHeapStatistics().heap_size_limit
0

Heap size of ZERO, when set to 4092.

This also happens on Mac OS X 10.11.6 and Node v5.11.0

@mscdex mscdex added process Issues and PRs related to the process subsystem. v8 engine Issues and PRs related to the V8 dependency. labels Dec 8, 2016
@bnoordhuis
Copy link
Member

#10186

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Dec 11, 2016
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: nodejs#10185
PR-URL: nodejs#10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: nodejs#10185
PR-URL: nodejs#10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 19, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: nodejs#10185
PR-URL: nodejs#10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 24, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: nodejs#10185
PR-URL: nodejs#10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: nodejs#10185
PR-URL: nodejs#10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Mar 7, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: #10185
PR-URL: #10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Mar 7, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: #10185
PR-URL: #10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: #10185
PR-URL: #10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB.  Use 64 bits floats instead, those
should last us a while.

Fixes: #10185
PR-URL: #10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

3 participants