diff options
author | bruening@google.com <bruening@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-14 05:34:54 +0000 |
---|---|---|
committer | bruening@google.com <bruening@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-14 05:34:54 +0000 |
commit | 333ddf5c81b76621d99b15b40d3cb9b80e274372 (patch) | |
tree | 46f5c1a0a06d74280b1bb343a1b37390261b1fd1 /base | |
parent | d87dfb808d4dbdaeb3ad6330ee8365b85dfe6f53 (diff) | |
download | chromium_src-333ddf5c81b76621d99b15b40d3cb9b80e274372.zip chromium_src-333ddf5c81b76621d99b15b40d3cb9b80e274372.tar.gz chromium_src-333ddf5c81b76621d99b15b40d3cb9b80e274372.tar.bz2 |
Revert 251178 "Trace event micro-optimization: use finer grained..."
due to data races raised on the tsan v2 bot.
BUG=343802
> Trace event micro-optimization: use finer grained lock.
>
> GetCategoryGroupEnabledInternal is a hotspot.
> It traverses an append-only list, so it's safe to use a finer-grained lock,
> and avoid it altogether for the "fast path", i.e., when the category is
> already known.
>
> BUG=
>
> Review URL: https://codereview.chromium.org/144423009
TBR=bulach@chromium.org
Review URL: https://codereview.chromium.org/163613006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251262 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base')
-rw-r--r-- | base/debug/trace_event_impl.cc | 66 |
1 files changed, 27 insertions, 39 deletions
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc index 5665208..ce25c38 100644 --- a/base/debug/trace_event_impl.cc +++ b/base/debug/trace_event_impl.cc @@ -100,8 +100,7 @@ const int g_category_categories_exhausted = 2; const int g_category_metadata = 3; const int g_category_trace_event_overhead = 4; const int g_num_builtin_categories = 5; -// Skip default categories. -base::subtle::AtomicWord g_category_index = g_num_builtin_categories; +int g_category_index = g_num_builtin_categories; // Skip default categories. // The name of the current thread. This is used to decide if the current // thread name has changed. We combine all the seen thread names into the @@ -1260,51 +1259,40 @@ const unsigned char* TraceLog::GetCategoryGroupEnabledInternal( const char* category_group) { DCHECK(!strchr(category_group, '"')) << "Category groups may not contain double quote"; - // The g_category_groups is append only, avoid using a lock for the fast path. - int current_category_index = base::subtle::NoBarrier_Load(&g_category_index); + AutoLock lock(lock_); + unsigned char* category_group_enabled = NULL; // Search for pre-existing category group. - for (int i = 0; i < current_category_index; ++i) { + for (int i = 0; i < g_category_index; i++) { if (strcmp(g_category_groups[i], category_group) == 0) { - return &g_category_group_enabled[i]; + category_group_enabled = &g_category_group_enabled[i]; + break; } } - unsigned char* category_group_enabled = NULL; - // This is the slow path: the lock is not held in the case above, so more - // than one thread could have reached here trying to add the same category. - // Only hold to lock when actually appending a new category, and - // check the categories groups again. - AutoLock lock(lock_); - for (int i = 0; i < g_category_index; ++i) { - if (strcmp(g_category_groups[i], category_group) == 0) { - return &g_category_group_enabled[i]; + if (!category_group_enabled) { + // Create a new category group + DCHECK(g_category_index < MAX_CATEGORY_GROUPS) << + "must increase MAX_CATEGORY_GROUPS"; + if (g_category_index < MAX_CATEGORY_GROUPS) { + int new_index = g_category_index++; + // Don't hold on to the category_group pointer, so that we can create + // category groups with strings not known at compile time (this is + // required by SetWatchEvent). + const char* new_group = strdup(category_group); + ANNOTATE_LEAKING_OBJECT_PTR(new_group); + g_category_groups[new_index] = new_group; + DCHECK(!g_category_group_enabled[new_index]); + // Note that if both included and excluded patterns in the + // CategoryFilter are empty, we exclude nothing, + // thereby enabling this category group. + UpdateCategoryGroupEnabledFlag(new_index); + category_group_enabled = &g_category_group_enabled[new_index]; + } else { + category_group_enabled = + &g_category_group_enabled[g_category_categories_exhausted]; } } - - // Create a new category group. - DCHECK(g_category_index < MAX_CATEGORY_GROUPS) << - "must increase MAX_CATEGORY_GROUPS"; - if (g_category_index < MAX_CATEGORY_GROUPS) { - int new_index = base::subtle::Acquire_Load(&g_category_index); - // Don't hold on to the category_group pointer, so that we can create - // category groups with strings not known at compile time (this is - // required by SetWatchEvent). - const char* new_group = strdup(category_group); - ANNOTATE_LEAKING_OBJECT_PTR(new_group); - g_category_groups[new_index] = new_group; - DCHECK(!g_category_group_enabled[new_index]); - // Note that if both included and excluded patterns in the - // CategoryFilter are empty, we exclude nothing, - // thereby enabling this category group. - UpdateCategoryGroupEnabledFlag(new_index); - category_group_enabled = &g_category_group_enabled[new_index]; - // Update the max index now. - base::subtle::Release_Store(&g_category_index, new_index + 1); - } else { - category_group_enabled = - &g_category_group_enabled[g_category_categories_exhausted]; - } return category_group_enabled; } |