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

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

Closed
rdominy opened this issue Dec 8, 2016 · 1 comment
Closed

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

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 bnoordhuis mentioned this issue Dec 8, 2016
Merged
@bnoordhuis
Copy link
Member

#10186

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Dec 11, 2016
@bnoordhuis
lib,src: support values > 4GB in heap statistics
aa77b76 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
@bnoordhuis @italoacasas
lib,src: support values > 4GB in heap statistics
0d6d6e0 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 19, 2017
@bnoordhuis @italoacasas
lib,src: support values > 4GB in heap statistics
f127e0c 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 24, 2017
@bnoordhuis @italoacasas
lib,src: support values > 4GB in heap statistics
fe3d00e 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
@bnoordhuis @italoacasas
lib,src: support values > 4GB in heap statistics
6e8d627 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
@algv algv mentioned this issue Feb 15, 2017
Closed
MylesBorins pushed a commit that referenced this issue Mar 7, 2017
@bnoordhuis @MylesBorins
lib,src: support values > 4GB in heap statistics
3981717 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 7, 2017
@bnoordhuis @MylesBorins
lib,src: support values > 4GB in heap statistics
7d71316 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
@bnoordhuis @MylesBorins
lib,src: support values > 4GB in heap statistics
a3ad63b 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
@bnoordhuis @MylesBorins
lib,src: support values > 4GB in heap statistics
fc2cd63 
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[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
process Issues and PRs related to the process subsystem. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mscdex @bnoordhuis @rdominy