Skip to content

Commit

Permalink
src: do not cache NumberOfHeapSpaces() globally
Browse files Browse the repository at this point in the history
While `NumberOfHeapSpaces()` currently returns a constant value,
that is not strictly guaranteed by the V8 API as far as I can tell.
Therefore, caching it globally does not seem appropriate.

(The motivation here is that this squelches warnings which are
produced by concurrency debugging tooling due to the apparent
race conditions when accessing the global variable.)

PR-URL: #20971
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and MylesBorins committed Jun 6, 2018
1 parent d84aa51 commit f79096a
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions src/node_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ static const size_t kHeapSpaceStatisticsPropertiesCount =
HEAP_SPACE_STATISTICS_PROPERTIES(V);
#undef V

// Will be populated in InitializeV8Bindings.
static size_t number_of_heap_spaces = 0;


void CachedDataVersionTag(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand All @@ -100,6 +97,7 @@ void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo<Value>& args) {
HeapSpaceStatistics s;
Isolate* const isolate = env->isolate();
double* buffer = env->heap_space_statistics_buffer();
size_t number_of_heap_spaces = env->isolate()->NumberOfHeapSpaces();

for (size_t i = 0; i < number_of_heap_spaces; i++) {
isolate->GetHeapSpaceStatistics(&s, i);
Expand Down Expand Up @@ -153,7 +151,7 @@ void Initialize(Local<Object> target,
Uint32::NewFromUnsigned(env->isolate(),
kHeapSpaceStatisticsPropertiesCount));

number_of_heap_spaces = env->isolate()->NumberOfHeapSpaces();
size_t number_of_heap_spaces = env->isolate()->NumberOfHeapSpaces();

// Heap space names are extracted once and exposed to JavaScript to
// avoid excessive creation of heap space name Strings.
Expand Down

0 comments on commit f79096a

Please sign in to comment.