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: 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>
  • Loading branch information
bnoordhuis authored and italoacasas committed Jan 18, 2017
1 parent 7dcd255 commit 0d6d6e0
Show file tree
Hide file tree
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
Expand Up @@ -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;
Expand All @@ -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 =
Expand Down
8 changes: 4 additions & 4 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,22 +316,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;
}
Expand Down
12 changes: 6 additions & 6 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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_;

Expand Down
12 changes: 6 additions & 6 deletions src/node_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()) *
Expand Down

0 comments on commit 0d6d6e0

Please sign in to comment.