From a3ad63b9b3a627a4c45c3db98e8a62856b1ad640 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 9 Dec 2016 00:17:14 +0100 Subject: [PATCH] lib,src: support values > 4GB in heap statistics 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: https://github.com/nodejs/node/issues/10185 PR-URL: https://github.com/nodejs/node/pull/10186 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas --- lib/v8.js | 4 ++-- src/env-inl.h | 8 ++++---- src/env.h | 12 ++++++------ src/node_v8.cc | 12 ++++++------ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/v8.js b/lib/v8.js index e78a2480ff..c1ca09dda8 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -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; @@ -28,7 +28,7 @@ const kHeapSizeLimitIndex = v8binding.kHeapSizeLimitIndex; // 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 = diff --git a/src/env-inl.h b/src/env-inl.h index 0981a09aeb..002683f890 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -396,22 +396,22 @@ inline std::vector* 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; } diff --git a/src/env.h b/src/env.h index 3b6c42064f..92859f7df9 100644 --- a/src/env.h +++ b/src/env.h @@ -458,11 +458,11 @@ class Environment { // List of id's that have been destroyed and need the destroy() cb called. inline std::vector* 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); @@ -578,8 +578,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_; diff --git a/src/node_v8.cc b/src/node_v8.cc index 43dd86c757..2d44da42a4 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -54,8 +54,8 @@ void UpdateHeapStatisticsArrayBuffer(const FunctionCallbackInfo& 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(s.name()); + double* const buffer = env->heap_statistics_buffer(); +#define V(index, name, _) buffer[index] = static_cast(s.name()); HEAP_STATISTICS_PROPERTIES(V) #undef V } @@ -65,13 +65,13 @@ void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo& 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(s.name()); + static_cast(s.name()); HEAP_SPACE_STATISTICS_PROPERTIES(V) #undef V } @@ -100,7 +100,7 @@ void InitializeV8Bindings(Local 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; @@ -146,7 +146,7 @@ void InitializeV8Bindings(Local 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()) *