AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[runtime] Make access to FLAG_runtime_stats atomic.

Browse Source 
Background tasks read this flag, which creates a data race. This patch
works around the data races by making the access to the flag atomic.

The actual fix will be to not mutate the flag.

Bug: chromium:794911
Change-Id: Idcf03b7a1037e876036918418ce989b420784428
Reviewed-on: https://chromium-review.googlesource.com/834508
Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50215}
This commit is contained in:
Ulan Degenbaev 2017-12-19 19:23:19 +01:00 committed by Commit Bot
parent 2203a37c5d
commit 42ac7fe04b
4 changed files with 19 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/counters-inl.h
View File

@ -15,7 +15,7 @@ void RuntimeCallTimer::Start(RuntimeCallCounter* counter,
DCHECK(!IsStarted());
counter_ = counter;
parent_.SetValue(parent);
if (FLAG_runtime_stats ==
if (base::AsAtomic32::Relaxed_Load(&FLAG_runtime_stats) ==
v8::tracing::TracingCategoryObserver::ENABLED_BY_SAMPLING) {
return;
}

3
src/heap/gc-tracer.cc
View File

@ -6,6 +6,7 @@
#include <cstdarg>
#include "src/base/atomic-utils.h"
#include "src/counters.h"
#include "src/heap/heap-inl.h"
#include "src/isolate.h"
@ -52,7 +53,7 @@ GCTracer::BackgroundScope::BackgroundScope(GCTracer* tracer, ScopeId scope)
: tracer_(tracer), scope_(scope), runtime_stats_enabled_(false) {
start_time_ = tracer_->heap_->MonotonicallyIncreasingTimeInMs();
// TODO(cbruni): remove once we fully moved to a trace-based system.
if (V8_LIKELY(!FLAG_runtime_stats)) return;
if (V8_LIKELY(!base::AsAtomic32::Relaxed_Load(&FLAG_runtime_stats))) return;
timer_.Start(&counter_, nullptr);
runtime_stats_enabled_ = true;
}

15
src/tracing/tracing-category-observer.cc
View File

@ -4,6 +4,7 @@
#include "src/tracing/tracing-category-observer.h"
#include "src/base/atomic-utils.h"
#include "src/flags.h"
#include "src/tracing/trace-event.h"
#include "src/v8.h"
@ -37,12 +38,16 @@ void TracingCategoryObserver::OnTraceEnabled() {
TRACE_EVENT_CATEGORY_GROUP_ENABLED(
TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats"), &enabled);
if (enabled) {
v8::internal::FLAG_runtime_stats |= ENABLED_BY_TRACING;
base::AsAtomic32::Relaxed_Store(
&v8::internal::FLAG_runtime_stats,
(v8::internal::FLAG_runtime_stats | ENABLED_BY_TRACING));
}
TRACE_EVENT_CATEGORY_GROUP_ENABLED(
TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats_sampling"), &enabled);
if (enabled) {
v8::internal::FLAG_runtime_stats |= ENABLED_BY_SAMPLING;
base::AsAtomic32::Relaxed_Store(
&v8::internal::FLAG_runtime_stats,
v8::internal::FLAG_runtime_stats | ENABLED_BY_SAMPLING);
}
TRACE_EVENT_CATEGORY_GROUP_ENABLED(TRACE_DISABLED_BY_DEFAULT("v8.gc_stats"),
&enabled);
@ -57,8 +62,10 @@ void TracingCategoryObserver::OnTraceEnabled() {
}
void TracingCategoryObserver::OnTraceDisabled() {
v8::internal::FLAG_runtime_stats &=
~(ENABLED_BY_TRACING | ENABLED_BY_SAMPLING);
base::AsAtomic32::Relaxed_Store(
&v8::internal::FLAG_runtime_stats,
v8::internal::FLAG_runtime_stats &
~(ENABLED_BY_TRACING | ENABLED_BY_SAMPLING));
v8::internal::FLAG_gc_stats &= ~ENABLED_BY_TRACING;
v8::internal::FLAG_ic_stats &= ~ENABLED_BY_TRACING;
}

8
test/unittests/counters-unittest.cc
View File

@ -4,6 +4,7 @@
#include <vector>
#include "src/base/atomic-utils.h"
#include "src/base/platform/time.h"
#include "src/counters-inl.h"
#include "src/counters.h"
@ -55,8 +56,9 @@ static base::TimeTicks RuntimeCallStatsTestNow() {
class RuntimeCallStatsTest : public TestWithNativeContext {
public:
RuntimeCallStatsTest() {
FLAG_runtime_stats =
v8::tracing::TracingCategoryObserver::ENABLED_BY_NATIVE;
base::AsAtomic32::Relaxed_Store(
&FLAG_runtime_stats,
v8::tracing::TracingCategoryObserver::ENABLED_BY_NATIVE);
// We need to set {time_} to a non-zero value since it would otherwise
// cause runtime call timers to think they are uninitialized.
Sleep(1);
@ -67,7 +69,7 @@ class RuntimeCallStatsTest : public TestWithNativeContext {
// Disable RuntimeCallStats before tearing down the isolate to prevent
// printing the tests table. Comment the following line for debugging
// purposes.
FLAG_runtime_stats = 0;
base::AsAtomic32::Relaxed_Store(&FLAG_runtime_stats, 0);
}
static void SetUpTestCase() {