Skip to content

Commit

Permalink
lib,src: support values > 4GB in heap statistics
Browse files Browse the repository at this point in the history
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>
bnoordhuis authored and italoacasas committed Jan 27, 2017
1 parent 9916ee8 commit 6e8d627
Showing 4 changed files with 18 additions and 18 deletions.
4 changes: 2 additions & 2 deletions lib/v8.js
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@ const v8binding = process.binding('v8');

// Properties for heap statistics buffer extraction.
const heapStatisticsBuffer =
new Uint32Array(v8binding.heapStatisticsArrayBuffer);
new Float64Array(v8binding.heapStatisticsArrayBuffer);
const kTotalHeapSizeIndex = v8binding.kTotalHeapSizeIndex;
const kTotalHeapSizeExecutableIndex = v8binding.kTotalHeapSizeExecutableIndex;
const kTotalPhysicalSizeIndex = v8binding.kTotalPhysicalSizeIndex;
@@ -31,7 +31,7 @@ const kPeakMallocedMemoryIndex = v8binding.kPeakMallocedMemoryIndex;

// Properties for heap space statistics buffer extraction.
const heapSpaceStatisticsBuffer =
new Uint32Array(v8binding.heapSpaceStatisticsArrayBuffer);
new Float64Array(v8binding.heapSpaceStatisticsArrayBuffer);
const kHeapSpaces = v8binding.kHeapSpaces;
const kNumberOfHeapSpaces = kHeapSpaces.length;
const kHeapSpaceStatisticsPropertiesCount =
8 changes: 4 additions & 4 deletions src/env-inl.h
Original file line number Diff line number Diff line change
@@ -329,22 +329,22 @@ inline std::vector<int64_t>* Environment::destroy_ids_list() {
return &destroy_ids_list_;
}

inline uint32_t* Environment::heap_statistics_buffer() const {
inline double* Environment::heap_statistics_buffer() const {
CHECK_NE(heap_statistics_buffer_, nullptr);
return heap_statistics_buffer_;
}

inline void Environment::set_heap_statistics_buffer(uint32_t* pointer) {
inline void Environment::set_heap_statistics_buffer(double* pointer) {
CHECK_EQ(heap_statistics_buffer_, nullptr); // Should be set only once.
heap_statistics_buffer_ = pointer;
}

inline uint32_t* Environment::heap_space_statistics_buffer() const {
inline double* Environment::heap_space_statistics_buffer() const {
CHECK_NE(heap_space_statistics_buffer_, nullptr);
return heap_space_statistics_buffer_;
}

inline void Environment::set_heap_space_statistics_buffer(uint32_t* pointer) {
inline void Environment::set_heap_space_statistics_buffer(double* pointer) {
CHECK_EQ(heap_space_statistics_buffer_, nullptr); // Should be set only once.
heap_space_statistics_buffer_ = pointer;
}
12 changes: 6 additions & 6 deletions src/env.h
Original file line number Diff line number Diff line change
@@ -469,11 +469,11 @@ class Environment {
// List of id's that have been destroyed and need the destroy() cb called.
inline std::vector<int64_t>* destroy_ids_list();

inline uint32_t* heap_statistics_buffer() const;
inline void set_heap_statistics_buffer(uint32_t* pointer);
inline double* heap_statistics_buffer() const;
inline void set_heap_statistics_buffer(double* pointer);

inline uint32_t* heap_space_statistics_buffer() const;
inline void set_heap_space_statistics_buffer(uint32_t* pointer);
inline double* heap_space_statistics_buffer() const;
inline void set_heap_space_statistics_buffer(double* pointer);

inline char* http_parser_buffer() const;
inline void set_http_parser_buffer(char* buffer);
@@ -581,8 +581,8 @@ class Environment {
&HandleCleanup::handle_cleanup_queue_> handle_cleanup_queue_;
int handle_cleanup_waiting_;

uint32_t* heap_statistics_buffer_ = nullptr;
uint32_t* heap_space_statistics_buffer_ = nullptr;
double* heap_statistics_buffer_ = nullptr;
double* heap_space_statistics_buffer_ = nullptr;

char* http_parser_buffer_;

12 changes: 6 additions & 6 deletions src/node_v8.cc
Original file line number Diff line number Diff line change
@@ -57,8 +57,8 @@ void UpdateHeapStatisticsArrayBuffer(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
HeapStatistics s;
env->isolate()->GetHeapStatistics(&s);
uint32_t* const buffer = env->heap_statistics_buffer();
#define V(index, name, _) buffer[index] = static_cast<uint32_t>(s.name());
double* const buffer = env->heap_statistics_buffer();
#define V(index, name, _) buffer[index] = static_cast<double>(s.name());
HEAP_STATISTICS_PROPERTIES(V)
#undef V
}
@@ -68,13 +68,13 @@ void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
HeapSpaceStatistics s;
Isolate* const isolate = env->isolate();
uint32_t* buffer = env->heap_space_statistics_buffer();
double* buffer = env->heap_space_statistics_buffer();

for (size_t i = 0; i < number_of_heap_spaces; i++) {
isolate->GetHeapSpaceStatistics(&s, i);
size_t const property_offset = i * kHeapSpaceStatisticsPropertiesCount;
#define V(index, name, _) buffer[property_offset + index] = \
static_cast<uint32_t>(s.name());
static_cast<double>(s.name());
HEAP_SPACE_STATISTICS_PROPERTIES(V)
#undef V
}
@@ -103,7 +103,7 @@ void InitializeV8Bindings(Local<Object> target,
"updateHeapStatisticsArrayBuffer",
UpdateHeapStatisticsArrayBuffer);

env->set_heap_statistics_buffer(new uint32_t[kHeapStatisticsPropertiesCount]);
env->set_heap_statistics_buffer(new double[kHeapStatisticsPropertiesCount]);

const size_t heap_statistics_buffer_byte_length =
sizeof(*env->heap_statistics_buffer()) * kHeapStatisticsPropertiesCount;
@@ -149,7 +149,7 @@ void InitializeV8Bindings(Local<Object> target,
UpdateHeapSpaceStatisticsBuffer);

env->set_heap_space_statistics_buffer(
new uint32_t[kHeapSpaceStatisticsPropertiesCount * number_of_heap_spaces]);
new double[kHeapSpaceStatisticsPropertiesCount * number_of_heap_spaces]);

const size_t heap_space_statistics_buffer_byte_length =
sizeof(*env->heap_space_statistics_buffer()) *

0 comments on commit 6e8d627

Please sign in to comment.